Skip to content

feat: flip OpenTracing bridge default to false, use OTEL for HTTP middleware #51

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.

Sign up for GitHub

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

Merged
ankurs merged 3 commits into from
Mar 28, 2026
Merged

feat: flip OpenTracing bridge default to false, use OTEL for HTTP middleware #51

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1601e3c
feat: flip OpenTracing bridge default to false, use OTEL for HTTP mid…
ankurs Mar 28, 2026
91b060a
fix: use RequestURI for HTTP target semconv attribute
ankurs Mar 28, 2026
e3ee126
chore: drop replace directives, bump tracing v0.2.0, errors v0.2.6, i…
ankurs Mar 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ type CB interface {
```

<a name="New"></a>
### func [New](<https://github.com/go-coldbrew/core/blob/main/core.go#L544>)
### func [New](<https://github.com/go-coldbrew/core/blob/main/core.go#L540>)

```go
func New(c config.Config) CB
Expand Down
7 changes: 4 additions & 3 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,10 @@ type Config struct {
// OTLPSamplingRatio is the ratio of traces to sample (0.0 to 1.0)
// 1.0 means sample all traces, 0.1 means sample 10% of traces
OTLPSamplingRatio float64 `envconfig:"OTLP_SAMPLING_RATIO" default:"0.2"`
// OTLPUseOpenTracingBridge determines whether to set up OpenTracing compatibility bridge
// This allows using existing OpenTracing instrumentation with OpenTelemetry
OTLPUseOpenTracingBridge bool `envconfig:"OTLP_USE_OPENTRACING_BRIDGE" default:"true"`
// Deprecated: OpenTracing bridge is provided for backwards compatibility only.
// New services should leave this false (the default). Set to true only if you
// have existing OpenTracing instrumentation that hasn't been migrated to OTEL.
OTLPUseOpenTracingBridge bool `envconfig:"OTLP_USE_OPENTRACING_BRIDGE" default:"false"`
}

// Validate checks the configuration for common misconfigurations and returns
Expand Down
52 changes: 24 additions & 28 deletions core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,18 @@
"time"

"github.com/go-coldbrew/core/config"
"github.com/go-coldbrew/interceptors"

Check failure on line 17 in core.go

View workflow job for this annotation

GitHub Actions / test 

github.com/go-coldbrew/interceptors@v0.1.12: replacement directory ../interceptors does not exist

Check failure on line 17 in core.go

View workflow job for this annotation

GitHub Actions / build 

github.com/go-coldbrew/interceptors@v0.1.12: replacement directory ../interceptors does not exist
"github.com/go-coldbrew/log"
"github.com/go-coldbrew/log/loggers"
"github.com/go-coldbrew/options"
grpc_opentracing "github.com/grpc-ecosystem/go-grpc-middleware/tracing/opentracing"
grpcprom "github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus"
"github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
"github.com/klauspost/compress/gzhttp"
"github.com/opentracing/opentracing-go"
"github.com/opentracing/opentracing-go/ext"
"github.com/prometheus/client_golang/prometheus/promhttp"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/propagation"
semconv "go.opentelemetry.io/otel/semconv/v1.12.0"
oteltrace "go.opentelemetry.io/otel/trace"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/insecure"
Expand Down Expand Up @@ -146,7 +147,7 @@
ServiceVersion: c.config.ReleaseName,
SamplingRatio: c.config.OTLPSamplingRatio,
Compression: c.config.OTLPCompression,
UseOpenTracingBridge: c.config.OTLPUseOpenTracingBridge,
UseOpenTracingBridge: c.config.OTLPUseOpenTracingBridge, //nolint:staticcheck // reading deprecated field for backward compat
Insecure: c.config.OTLPInsecure,
}
if err := SetupOpenTelemetry(otlpConfig); err != nil {
Expand All @@ -166,33 +167,30 @@
}
}

// https://grpc-ecosystem.github.io/grpc-gateway/docs/operations/tracing/#opentracing-support
var grpcGatewayTag = opentracing.Tag{Key: string(ext.Component), Value: "grpc-gateway"}

// tracingWrapper is a middleware that creates a new span for each incoming request.
// It also adds the span to the context so it can be used by other middlewares or handlers to add additional tags.
// tracingWrapper is a middleware that creates a new OTEL span for each incoming HTTP request.
// It extracts any propagated trace context from the request headers and, for non-filtered
// methods, starts a server span that is attached to the request context.
func tracingWrapper(h http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
parentSpanContext, err := opentracing.GlobalTracer().Extract(
opentracing.HTTPHeaders,
opentracing.HTTPHeadersCarrier(r.Header))
if err == nil || err == opentracing.ErrSpanContextNotFound {
if interceptors.FilterMethodsFunc(r.Context(), r.URL.Path) {
serverSpan := opentracing.GlobalTracer().StartSpan(
"ServeHTTP",
// this is magical, it attaches the new span to the parent parentSpanContext, and creates an unparented one if empty.
ext.RPCServerOption(parentSpanContext),
grpcGatewayTag,
opentracing.Tag{Key: string(ext.HTTPUrl), Value: r.URL.Path},
opentracing.Tag{Key: string(ext.HTTPMethod), Value: r.Method},
)
r = r.WithContext(opentracing.ContextWithSpan(r.Context(), serverSpan))
defer serverSpan.Finish()
}
prop := otel.GetTextMapPropagator()
ctx := prop.Extract(r.Context(), propagation.HeaderCarrier(r.Header))
if interceptors.FilterMethodsFunc(ctx, r.URL.Path) {
var serverSpan oteltrace.Span
ctx, serverSpan = otel.Tracer("coldbrew-http").Start(ctx, "ServeHTTP",
oteltrace.WithSpanKind(oteltrace.SpanKindServer),
oteltrace.WithAttributes(
semconv.HTTPMethodKey.String(r.Method),
semconv.HTTPTargetKey.String(r.URL.RequestURI()),
),
)
r = r.WithContext(ctx)
defer serverSpan.End()
} else {
r = r.WithContext(ctx)
}
Comment thread
ankurs marked this conversation as resolved.
_, han := interceptors.NRHttpTracer("", h.ServeHTTP)
// add this info to log
ctx := r.Context()
ctx = r.Context()
ctx = options.AddToOptions(ctx, "", "")
ctx = loggers.AddToLogContext(ctx, "httpPath", r.URL.Path)
r = r.WithContext(ctx)
Expand Down Expand Up @@ -259,8 +257,6 @@
grpc.WithTransportCredentials(creds),
grpc.WithUnaryInterceptor(
interceptors.DefaultClientInterceptor(
grpc_opentracing.WithTraceHeaderName(c.config.TraceHeaderName),
grpc_opentracing.WithFilterFunc(interceptors.FilterMethodsFunc),
interceptors.WithoutHystrix(),
),
),
Expand Down
9 changes: 7 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ require (
github.com/go-coldbrew/options v0.2.6
github.com/go-coldbrew/tracing v0.0.7
github.com/golang/protobuf v1.5.4
github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.1.0
github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0
github.com/klauspost/compress v1.18.5
Expand All @@ -23,6 +22,7 @@ require (
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.42.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.42.0
go.opentelemetry.io/otel/sdk v1.42.0
go.opentelemetry.io/otel/trace v1.42.0
go.uber.org/automaxprocs v1.6.0
google.golang.org/grpc v1.79.3
google.golang.org/protobuf v1.36.11
Expand Down Expand Up @@ -262,7 +262,6 @@ require (
go.augendre.info/fatcontext v0.9.0 // indirect
go.opentelemetry.io/auto/sdk v1.2.1 // indirect
go.opentelemetry.io/otel/metric v1.42.0 // indirect
go.opentelemetry.io/otel/trace v1.42.0 // indirect
go.opentelemetry.io/proto/otlp v1.10.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.27.1 // indirect
Expand Down Expand Up @@ -296,3 +295,9 @@ tool (
github.com/princjef/gomarkdoc/cmd/gomarkdoc
golang.org/x/vuln/cmd/govulncheck
)

replace github.com/go-coldbrew/tracing => ../tracing

replace github.com/go-coldbrew/interceptors => ../interceptors

replace github.com/go-coldbrew/errors => ../errors
Comment thread
ankurs marked this conversation as resolved.
Outdated

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

@ankurs ankurs Mar 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Loading
Loading