Add OpenTelemetry trace context propagation#99
Conversation
|
Warning Review limit reached
More reviews will be available in 38 minutes and 42 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (24)
📝 WalkthroughWalkthroughThe PR adds trace-context propagation helpers, observability manifest types, runtime tracing spans, and scaffold support for metrics and tracing. It also updates generated GitOps output, docs, and transport tests to carry and validate trace metadata. ChangesObservability and scaffolded tracing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
distributed_cli/src/generate/service_crate.rs (1)
82-132: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: collapse the duplicated metrics
mainbranches.The
Http if self.metricsandKnative if self.metricsarms are identical except forroutervscloud_events_router. The body (listener bind,/metricsroute,axum::serve,metrics()handler) could be generated once with the router constructor selected by transport.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@distributed_cli/src/generate/service_crate.rs` around lines 82 - 132, The metrics-enabled HTTP and Knative branches in service_crate::generate are duplicating the same main/metrics handler logic. Refactor the ServiceTransport::Http if self.metrics and ServiceTransport::Knative if self.metrics arms to share one template, and make only the router constructor differ between distributed::microsvc::router and distributed::microsvc::cloud_events_router while keeping the listener bind, /metrics route, axum::serve call, and metrics() output in one generated body.distributed_cli/src/cli.rs (1)
74-74: 📐 Maintainability & Code Quality | 🔵 TrivialUse
visible_aliasfor the--otelflag to ensure it appears in--help.The
alias = "otel"attribute creates a functional alias that works at runtime but hides it from the help output. Since the README documents--otelas a supported flag, users following the documentation and then running--helpwill not find it listed.- #[arg(long, alias = "otel")] + #[arg(long, visible_alias = "otel")]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@distributed_cli/src/cli.rs` at line 74, The `--otel` flag alias on the CLI argument is currently hidden from help output because `alias = "otel"` is used in the `cli.rs` argument definition. Update the `#[arg(...)]` on the affected option in `distributed_cli::cli::Cli` to use `visible_alias = "otel"` instead, so the flag remains functional and is shown in `--help`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@distributed_cli/src/generate/gitops.rs`:
- Around line 119-131: The OTEL_SERVICE_NAME env var in the gitops template is
using the deploy chart name from .Chart.Name, which produces the wrong service
identity for traces. Update the env block in the generator that builds the Helm
snippet to inject the package/service name already used elsewhere in this
codepath (matching ServiceManifest::new and the service label), instead of
hardcoding .Chart.Name, so trace and metric identities stay aligned.
In `@distributed_cli/src/generate/service_crate.rs`:
- Around line 53-55: The generated main entrypoint is missing tracing setup, so
spans are never exported even when the otel feature is enabled. Update
service_crate.rs in main_rs to conditionally emit the opentelemetry OTLP
exporter initialization from OTEL_EXPORTER_OTLP_ENDPOINT and install the
matching tracing_subscriber layer when self.tracing is true. Make sure the
generated main function performs this setup before calling
distributed::microsvc::serve, and keep the logic gated so non-tracing builds
remain unchanged.
In `@src/microsvc/knative_ingress.rs`:
- Around line 96-113: The trace context update in
parse_cloud_event/inject_http_trace_context is unconditional and can wipe
existing metadata from parse_binary or parse_structured when HTTP trace headers
are absent. Add a guard in inject_http_trace_context so it only calls
TraceContext::inject_vec when at least one of TRACEPARENT or TRACESTATE is
present in the HeaderMap, otherwise leave the Message.metadata unchanged.
In `@src/trace_context.rs`:
- Around line 101-109: The is_valid_traceparent validator currently accepts any
non-ff version and uppercase hex, but it should enforce the documented
version-00 shape. Update the checks in is_valid_traceparent (and the related
logic around the traceparent parsing/validation path) to require version == "00"
and to only accept lowercase hex for version-00 fields, while keeping the
existing length and format validation for trace_id, parent_id, and flags.
---
Nitpick comments:
In `@distributed_cli/src/cli.rs`:
- Line 74: The `--otel` flag alias on the CLI argument is currently hidden from
help output because `alias = "otel"` is used in the `cli.rs` argument
definition. Update the `#[arg(...)]` on the affected option in
`distributed_cli::cli::Cli` to use `visible_alias = "otel"` instead, so the flag
remains functional and is shown in `--help`.
In `@distributed_cli/src/generate/service_crate.rs`:
- Around line 82-132: The metrics-enabled HTTP and Knative branches in
service_crate::generate are duplicating the same main/metrics handler logic.
Refactor the ServiceTransport::Http if self.metrics and
ServiceTransport::Knative if self.metrics arms to share one template, and make
only the router constructor differ between distributed::microsvc::router and
distributed::microsvc::cloud_events_router while keeping the listener bind,
/metrics route, axum::serve call, and metrics() output in one generated body.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aacf9019-2f39-41e1-8653-f91da7554bfa
📒 Files selected for processing (24)
Cargo.tomldistributed_cli/README.mddistributed_cli/src/cli.rsdistributed_cli/src/generate/gitops.rsdistributed_cli/src/generate/mod.rsdistributed_cli/src/generate/service_crate.rsdistributed_cli/src/lib.rsdocs/observability.mddocs/transports.mdsrc/bus/message.rssrc/bus/runner.rssrc/entity/entity.rssrc/entity/event_record.rssrc/lib.rssrc/manifest.rssrc/microsvc/knative_ingress.rssrc/microsvc/service.rssrc/outbox/message.rssrc/outbox_worker/outbox_dispatch.rssrc/trace_context.rstests/kafka_transport/main.rstests/nats_transport/main.rstests/persistent_repository_conformance/scenario.rstests/rabbitmq_transport/main.rs
patrickleet
force-pushed
the
codex/opentelemetry-tracing-compatibility
branch
from
b07d1eb to
6d43624
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@distributed_cli/README.md`:
- Around line 25-30: Remove the duplicated `--gitops` entry from the options
list in the README help text so the `dctl scaffold` flags are listed only once.
Update the sentence that enumerates common flags around `--gitops`, `--metrics`,
and `--tracing` to keep the list accurate and avoid implying multiple distinct
`--gitops` switches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad3470a8-d2c1-499f-8dc1-f77e2428607e
📒 Files selected for processing (24)
Cargo.tomldistributed_cli/README.mddistributed_cli/src/cli.rsdistributed_cli/src/generate/gitops.rsdistributed_cli/src/generate/mod.rsdistributed_cli/src/generate/service_crate.rsdistributed_cli/src/lib.rsdocs/observability.mddocs/transports.mdsrc/bus/message.rssrc/bus/runner.rssrc/entity/entity.rssrc/entity/event_record.rssrc/lib.rssrc/manifest.rssrc/microsvc/knative_ingress.rssrc/microsvc/service.rssrc/outbox/message.rssrc/outbox_worker/outbox_dispatch.rssrc/trace_context.rstests/kafka_transport/main.rstests/nats_transport/main.rstests/persistent_repository_conformance/scenario.rstests/rabbitmq_transport/main.rs
✅ Files skipped from review due to trivial changes (3)
- docs/observability.md
- docs/transports.md
- Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (20)
- tests/kafka_transport/main.rs
- distributed_cli/src/cli.rs
- src/outbox_worker/outbox_dispatch.rs
- tests/persistent_repository_conformance/scenario.rs
- src/lib.rs
- distributed_cli/src/lib.rs
- tests/rabbitmq_transport/main.rs
- src/outbox/message.rs
- distributed_cli/src/generate/gitops.rs
- src/entity/entity.rs
- src/microsvc/service.rs
- tests/nats_transport/main.rs
- src/microsvc/knative_ingress.rs
- src/trace_context.rs
- src/bus/runner.rs
- src/bus/message.rs
- distributed_cli/src/generate/service_crate.rs
- distributed_cli/src/generate/mod.rs
- src/entity/event_record.rs
- src/manifest.rs
patrickleet
force-pushed
the
codex/opentelemetry-tracing-compatibility
branch
from
6d43624 to
711ce33
Compare
Implements [[tasks/opentelemetry-tracing-compatibility]]
patrickleet
force-pushed
the
codex/opentelemetry-tracing-compatibility
branch
from
711ce33 to
258708c
Compare
|
CodeRabbit summary nitpick follow-up ( |
|
CodeRabbit summary nitpick follow-up (duplicated metrics |
1 participant
Summary
otelfeature that emits framework-owned tracing spans without requiring an OpenTelemetry SDK in the default builddsvc scaffold --metrics --tracing/--otelGitOps output for OTLP env values and HTTP ServiceMonitor generationVerification
cargo fmt --checkgit diff --checkcargo test --all-featurescargo clippy --lib --all-features -- -D warningscargo test -p distributed_clicargo clippy -p distributed_cli --all-targets -- -D warningscargo run -p distributed_cli --bin dsvc -- scaffold observability-orders --path target/tmp/scaffold-observability --store in-memory --transport http --metrics --tracing --gitops --force --distributed-path .cargo check --manifest-path target/tmp/scaffold-observability/Cargo.tomlImplements [[tasks/opentelemetry-tracing-compatibility]].
Summary by CodeRabbit
metrics(HTTP/metrics+ Prometheus scrape artifacts) andtracing/otel(OTLP-enabled spans + Helm/GitOps env templating).traceparent/tracestate) across messages, entities, event records, and outbox messages.