feat: Add structured internal logging for debugging for Geneva-uploader #472
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@ ## main #472 +/- ## ======================================= - Coverage 54.0% 53.6% -0.5% ======================================= Files 71 71 Lines 11501 11632 +131 ======================================= + Hits 6220 6244 +24 - Misses 5281 5388 +107 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lalitb changed the title There was a problem hiding this comment.
Pull Request Overview
This PR introduces structured internal logging for debugging across the Geneva uploader components using the tracing crate. It adds info-level logs for lifecycle events (initialization, auth token acquisition) and debug-level logs for hot-path operations (encoding, compression, uploads).
- Added comprehensive tracing throughout config client, token workflows, encoding, and upload pipeline
- Updated the basic example with detailed documentation on how to enable geneva-uploader internal logs
- Enhanced RUST_LOG environment variable handling in the example to merge user-specified directives with defaults
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| opentelemetry-exporter-geneva/examples/basic.rs | Added documentation and improved RUST_LOG handling for geneva-uploader internal logs |
| geneva-uploader/src/payload_encoder/otlp_encoder.rs | Added debug logging for encoding operations, compression errors, and batch statistics |
| geneva-uploader/src/ingestion_service/uploader.rs | Added debug logging for upload lifecycle, success/failure cases, and error handling |
| geneva-uploader/src/config_service/client.rs | Added comprehensive logging for auth workflows, cache operations, and config service interactions |
| geneva-uploader/src/client.rs | Added info/debug logging for client initialization and batch processing operations |
| geneva-uploader/Cargo.toml | Added tracing dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
opentelemetry-exporter-geneva/geneva-uploader/src/config_service/client.rs Outdated Show resolved Hide resolved
| debug!( | ||
| name: "config_client.new.certificate_parse_error", | ||
| target: "geneva-uploader", | ||
| "Failed to parse PKCS#12 certificate: {}", e |
There was a problem hiding this comment.
The debug! macro should use structured logging format instead of string formatting. Use error = %e in the macro arguments and remove the {} placeholder from the message.
opentelemetry-exporter-geneva/geneva-uploader/src/config_service/client.rs Outdated Show resolved Hide resolved
opentelemetry-exporter-geneva/geneva-uploader/src/config_service/client.rs Outdated Show resolved Hide resolved
opentelemetry-exporter-geneva/geneva-uploader/src/config_service/client.rs Outdated Show resolved Hide resolved
opentelemetry-exporter-geneva/opentelemetry-exporter-geneva/examples/basic.rs Outdated Show resolved Hide resolved
There was a problem hiding this comment.
Approving as the changes are helpful.
A few things to note though:
- I think we don't have consistency in the usage of
debug!vsinfo!. I believe we want the hot-path operations to usedebug!andinfo!for everything else. We don't seem to follow that - Use structured logging as suggested by Copilot
- The examples could simply add a directive for the
geneva_uploadertarget instead of relying on the environment variable.
Thanks for reviewing/ We want to
If this is not consistent, I will fix it. |
…ce/client.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ce/client.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ce/client.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ce/client.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ce/client.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ce/client.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ce/client.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| @utpilla All the comments are resolved. Will be merging now, if anything left, will accommodate in next release. Thanks for reviewing. |
2 participants
Changes
Introduce consistent, structured tracing across the Geneva uploader (config client, token workflows, encoding, upload pipeline) using
Update the opentelemetry-exporter-geneva example too to demonstrate it:
debug-logs:
info-logs:
Please provide a brief description of the changes here.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes