-
Notifications
You must be signed in to change notification settings - Fork 522
Tweaks to logs integration test #2453
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
Tweaks to logs integration test #2453
Conversation
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.
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
Files not reviewed (2)
- opentelemetry-otlp/tests/integration_test/expected/failed_logs.json: Language not supported
- opentelemetry-otlp/tests/integration_test/expected/logs.json: Language not supported
info!(target: "my-target", "hello from {}. My price is {}.", "banana", 2.99); | ||
} | ||
// TODO: remove below wait before calling logger_provider.shutdown() | ||
// tokio::time::sleep(Duration::from_secs(10)).await; |
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.
Commented-out code should be removed or properly handled. Leaving it commented out with a 'TODO' is not ideal.
// tokio::time::sleep(Duration::from_secs(10)).await; | |
// let _ = logger_provider.shutdown(); |
Copilot uses AI. Check for mistakes.
} | ||
} | ||
} | ||
}); | ||
}).expect("Thread spawn failed."); //TODO: Handle thread spawn failure |
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.
The error message for thread spawn failure is not descriptive enough. Consider providing more details, such as: 'Failed to spawn OpenTelemetry logs batch processor thread.'
}).expect("Thread spawn failed."); //TODO: Handle thread spawn failure | |
}).expect("Failed to spawn OpenTelemetry logs batch processor thread."); |
Copilot uses AI. Check for mistakes.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2453 +/- ##
=====================================
Coverage 76.1% 76.1%
=====================================
Files 122 122
Lines 22062 22065 +3
=====================================
+ Hits 16790 16793 +3
Misses 5272 5272 ☔ View full report in Codecov by Sentry. |
Follow up to #2436
Replaces
log
crate withtracing
crate in integration tests.Name the spawned thread in BatchLogProcessor.
Validate Scope too in integration tests.