Skip to content
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

Merged
merged 2 commits into from
Apr 19, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)");
Copy link
Member Author

@trask trask Mar 1, 2024

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

Copy link
Contributor

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.

Copy link
Member Author

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

} else {
telemetryPipelineListener =
TelemetryPipelineListener.composite(
// suppress warnings on retryable failures, in order to reduce sporadic/annoying
Copy link
Contributor

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

Copy link
Member Author

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?

// warnings when storing to disk and retrying shortly afterwards anyways
// 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)"),
"Sending telemetry to the ingestion service", false, ""),
new LocalStorageTelemetryPipelineListener(
diskPersistenceMaxSizeMb,
TempDirs.getSubDir(tempDir, TELEMETRY_FOLDER_NAME),
Expand Down