-
Notifications
You must be signed in to change notification settings - Fork 199
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
Reduce logging #3564
Reduce logging #3564
Conversation
@@ -236,7 +236,7 @@ private BatchItemProcessor initBatchItemProcessor( | |||
if (tempDir == null) { | |||
telemetryPipelineListener = | |||
new DiagnosticTelemetryPipelineListener( | |||
"Sending telemetry to the ingestion service", false, " (telemetry will be lost)"); | |||
"Sending telemetry to the ingestion service", true, " (telemetry will be lost)"); |
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.
this looks like a bug, 3rd arg isn't even used when 2nd arg is false
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.
tempDir is null.. written to disk is not possible. retries will be false. i don't get it why it becomes true.
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.
2nd arg controls logging, not writing to disk
@@ -245,8 +245,8 @@ private BatchItemProcessor initBatchItemProcessor( | |||
// will log if that retry from disk fails | |||
new DiagnosticTelemetryPipelineListener( | |||
"Sending telemetry to the ingestion service", | |||
true, | |||
" (telemetry will be stored to disk and retried)"), | |||
false, |
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.
not get it why it's false here.
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.
so that we only log the 2nd time a batch fails (when retrying from disk)
@@ -236,17 +236,15 @@ private BatchItemProcessor initBatchItemProcessor( | |||
if (tempDir == null) { | |||
telemetryPipelineListener = | |||
new DiagnosticTelemetryPipelineListener( | |||
"Sending telemetry to the ingestion service", false, " (telemetry will be lost)"); | |||
"Sending telemetry to the ingestion service", true, " (telemetry will be lost)"); | |||
} else { | |||
telemetryPipelineListener = | |||
TelemetryPipelineListener.composite( | |||
// suppress warnings on retryable failures, in order to reduce sporadic/annoying |
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.
please update the comment.. this is not what it's actually doing
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.
I think the comment is correct, can you double check?
No description provided.