-
Notifications
You must be signed in to change notification settings - Fork 178
feat: allow OTLP exporting by extending existing logger configs #835
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: develop
Are you sure you want to change the base?
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.
Pull request overview
This PR adds OTLP (OpenTelemetry Protocol) distributed tracing support by extending the existing logger configuration infrastructure. The changes introduce OtlpConfig and TracingConfig types that wrap the existing LoggerConfig, allowing applications to optionally enable OTLP exporting alongside local logging.
Key Changes:
- New
OtlpConfigandTracingConfigtypes for unified logging and tracing configuration - Updated initialization code across multiple binaries to use the new tracing configuration
- Added OTLP dependencies to the rbuilder-config crate
Reviewed changes
Copilot reviewed 9 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rbuilder-config/src/otlp.rs | Implements OTLP configuration and guard for managing the tracer provider lifecycle |
| crates/rbuilder-config/src/tracing.rs | Defines unified tracing configuration that combines logging and OTLP |
| crates/rbuilder-config/src/logger.rs | Refactors logger initialization and deprecates direct init_tracing method |
| crates/rbuilder-config/src/lib.rs | Exports new OTLP and tracing modules |
| crates/rbuilder-config/Cargo.toml | Adds OpenTelemetry dependencies |
| crates/rbuilder/src/live_builder/base_config.rs | Updates to use TracingConfig with optional OTLP support |
| crates/test-relay/src/main.rs | Migrates CLI to use TracingConfig and adds OTLP environment name option |
| crates/rbuilder-rebalancer/src/config.rs | Renames logger field to tracing and updates to use TracingConfig |
| crates/bid-scraper/src/config.rs | Adds otlp_env_name field to configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// # Panics | ||
| /// | ||
| /// If `self.log_json` is true. | ||
| pub(crate) fn ansi(&self) -> eyre::Result<SubscriberBuilder<DefaultFields, Format, EnvFilter>> { |
Copilot
AI
Dec 16, 2025
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 return type 'Format' is ambiguous and should be fully qualified as 'Format<Full, SystemTime>' to match the pattern used in the 'builder()' method.
| pub(crate) fn ansi(&self) -> eyre::Result<SubscriberBuilder<DefaultFields, Format, EnvFilter>> { | |
| pub(crate) fn ansi(&self) -> eyre::Result<SubscriberBuilder<DefaultFields, Format<Full, SystemTime>, EnvFilter>> { |
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 go the other way. including default type args you don't have to is overly verbose
📝 Summary
Add simple OTLP exporting by extending existing logger configs.
The goal is to allow rbuilder to export spans, events, and other trace information to tools like zipkin or groundcover
💡 Motivation and Context
Distributed tracing is important for production systems :)
✅ I have completed the following steps:
make lintmake test