-
Notifications
You must be signed in to change notification settings - Fork 482
Suppress additional polling logs #11947
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,8 +23,12 @@ public static class ScriptLoggingBuilderExtensions | |
| // suppressed to reduce noise in the FunctionsLogs table. | ||
| private static readonly HashSet<string> _suppressedCategories = new(StringComparer.Ordinal) | ||
| { | ||
| "Host.Triggers.Kafka", | ||
| "Microsoft.Azure.WebJobs.EventHubs.Listeners.EventHubListener.PartitionProcessor", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I notice that for EventHub, you're not suppressing Microsoft.Azure.WebJobs.EventHubs.Listeners.EventHubListener. If we're inconsistent across extensions wrt. suppression of their listener start/stop lifecycle events as we will be with these changes, I wonder if that will cause confusion - we'll no longer know which extensions we can count on these logs for in investigations. |
||
| "Microsoft.Azure.WebJobs.Extensions.Storage.Common.Listeners.QueueListener", | ||
| "Microsoft.Azure.WebJobs.Extensions.Storage.Blobs.Listeners.BlobListener", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logger for this listener is here https://github.com/Azure/azure-sdk-for-net/blob/ca61f202024f99d33f4693fe3176d54092f41205/sdk/storage/Microsoft.Azure.WebJobs.Extensions.Storage.Blobs/src/Listeners/BlobListener.cs#L35 and appears only to be used to log listener start/stop lifecycle events. Those are important and also aren't high volume, so why do we want to suppress them? Surveying prod logs, I see the following event names being logged using this source: "BlobAlreadyProcessed", "PollBlobContainer", "BlobDoesNotMatchPattern", "BlobMessageEnqueued", "ContainerDoesNotExist", "InitializedScanInfo", "ScanBlobLogs", "FunctionNotFound", "BlobHasNoETag" Some of those are clearly noisy, but some are useful for debugging and infrequent. I worry that simply throwing all of these away is going to hamper our ability to investigate issues. |
||
| "Microsoft.Azure.WebJobs.EventHubs.EventHubProducerClientImpl", | ||
| "Microsoft.Azure.WebJobs.Host.Queues.Listeners.QueueListener", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you double-check what we'll lose from this? Is it just the polling logs? We should still have the option to do a {
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think the category/eventname approach would be safer. All of the listener start/stop lifecycle logs appear to be logged with this category, e.g. https://github.com/Azure/azure-sdk-for-net/blob/ca61f202024f99d33f4693fe3176d54092f41205/sdk/storage/Microsoft.Azure.WebJobs.Extensions.Storage.Common/src/Shared/Queues/QueueListener.cs#L173. We don't want to lose those.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah subtle point here - this source you're adding Microsoft.Azure.WebJobs.Host.Queues.Listeners.QueueListener is for older versions of the extension - the current QueueListener in newer storage extension is in namespace Microsoft.Azure.WebJobs.Extensions.Storage.Common.Listeners.QueueListener. The code you're modifying here already filters Microsoft.Azure.WebJobs.Extensions.Storage.Common.Listeners.QueueListener - has that been released yet? We don't want to be filtering all these QueueListener debug logs out |
||
| "Host.Executor" | ||
| }; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tsuyoshi Ushio (@TsuyoshiUshio) to review what types of logs we emit using this category and whether anything important would be lost