feat: flip OpenTracing bridge default to false, use OTEL for HTTP middleware#51
feat: flip OpenTracing bridge default to false, use OTEL for HTTP middleware#51
Conversation
…dleware - Flip OTLPUseOpenTracingBridge default from true to false (deprecated) - Replace OpenTracing HTTP middleware with native OTEL tracer + propagator - Remove grpc_opentracing options from gRPC gateway client setup - Keep bridge code in initializers.go for backward compatibility - Replace directives for local tracing/interceptors/errors (remove before merge)
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 7 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Updates tracing defaults and middleware to prefer native OpenTelemetry over OpenTracing, while keeping an opt-in OpenTracing bridge for backward compatibility.
Changes:
- Flips
OTLPUseOpenTracingBridgedefault tofalseand marks it deprecated for backward compatibility. - Replaces the HTTP tracing middleware implementation from OpenTracing to OTEL (
otel.Tracer().Start+ global propagator extraction). - Removes OpenTracing-related gRPC gateway client options and drops the
go-grpc-middlewarev1 dependency.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
go.mod |
Removes gRPC opentracing middleware dependency, adds direct OTEL trace dep, and introduces local replace directives (temporary). |
go.sum |
Cleans up sums consistent with dependency changes. |
core.go |
Replaces OpenTracing HTTP middleware with OTEL spans + propagator extraction; removes gRPC opentracing options for gateway dial. |
config/config.go |
Changes OTLPUseOpenTracingBridge default to false and documents deprecation/back-compat intent. |
README.md |
Updates generated link target line number for New. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| replace github.com/go-coldbrew/tracing => ../tracing | ||
|
|
||
| replace github.com/go-coldbrew/interceptors => ../interceptors | ||
|
|
||
| replace github.com/go-coldbrew/errors => ../errors |
There was a problem hiding this comment.
The local replace directives are intended to be temporary; when they’re removed, this go.mod will fall back to the currently pinned versions (github.com/go-coldbrew/{tracing,interceptors,errors}), which likely won’t include the OTEL migration changes this PR depends on. Before dropping the replace lines, update the require versions to the tagged releases that include the upstream PRs and re-run go mod tidy so builds don’t silently regress to older behavior or fail to compile.
There was a problem hiding this comment.
Correct — already noted in the PR description. When dropping the replace directives, the require versions will be bumped to the tagged releases containing the OTEL migration changes from tracing#20, errors#18, and interceptors#25.
…nterceptors v0.1.13
Summary
OTLPUseOpenTracingBridgedefault fromtruetofalse(deprecated, kept for backward compat)tracingWrapper) with native OTEL:otel.Tracer().Start()+otel.GetTextMapPropagator()grpc_opentracingoptions from gRPC gateway client setupSetupOpenTelemetryfor services that still need it (OTLP_USE_OPENTRACING_BRIDGE=true)Replace directives
Contains temporary replace directives for local tracing/interceptors/errors packages. Remove before merging — wait for upstream PRs to land first.
Depends on
Migration for users
Services relying on the implicit OpenTracing bridge should set
OTLP_USE_OPENTRACING_BRIDGE=trueexplicitly.Test plan
go test -race ./...— passes (with local replace)make lint— 0 issues