Skip to content

Conversation

@prestwich
Copy link

@prestwich prestwich commented Dec 16, 2025

📝 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:

  • Run make lint
  • Run make test
  • Added tests (if applicable)

Copilot AI review requested due to automatic review settings December 16, 2025 16:39
Copy link
Contributor

Copilot AI left a 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 OtlpConfig and TracingConfig types 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>> {
Copy link

Copilot AI Dec 16, 2025

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.

Suggested change
pub(crate) fn ansi(&self) -> eyre::Result<SubscriberBuilder<DefaultFields, Format, EnvFilter>> {
pub(crate) fn ansi(&self) -> eyre::Result<SubscriberBuilder<DefaultFields, Format<Full, SystemTime>, EnvFilter>> {

Copilot uses AI. Check for mistakes.
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant