Skip to content
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

config: add support for configuring propagators #6727

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
+287 −6
Open

config: add support for configuring propagators #6727

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d09831f
config: add support for configuring propagators
jpkrohling Feb 6, 2025
2d858e6
make linter happy
jpkrohling Feb 6, 2025
5ec3c25
empty vs. nil
jpkrohling Feb 6, 2025
a73e9ea
changelog, err handling, moved propagators logic to before signals
jpkrohling Feb 6, 2025
df3d0af
reuse autoprop
jpkrohling Feb 7, 2025
88928e6
make linter happy
jpkrohling Feb 7, 2025
c485443
add config to propagator
jpkrohling Feb 7, 2025
4cc8205
str pointers
jpkrohling Feb 7, 2025
c6af81b
revert changes to propagators
jpkrohling Feb 15, 2025
e40d6df
crosslink and be more strict with nil/empty
jpkrohling Feb 15, 2025
7a9f116
go.sum
jpkrohling Feb 15, 2025
b61af11
review comments
jpkrohling Feb 18, 2025
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ The next release will require at least [Go 1.23].
- Added metrics support, and emit all stable metrics from the [Semantic Conventions](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-metrics.md) in `go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux`. (#6648)
- Add support for configuring `Insecure` field for OTLP exporters in `go.opentelemetry.io/contrib/config`. (#6658)
- Support for the `OTEL_HTTP_CLIENT_COMPATIBILITY_MODE=http/dup` environment variable in `instrumentation/net/http/httptrace/otelhttptrace` to emit attributes for both the v1.20.0 and v1.26.0 semantic conventions. (#6720)
- Add support for configuring propagators in `go.opentelemetry.io/contrib/config`. (#6727)
- Support for the `OTEL_HTTP_CLIENT_COMPATIBILITY_MODE=http/dup` environment variable in `instrumentation/github.com/emicklei/go-restful/otelrestful` to emit attributes for both the v1.20.0 and v1.26.0 semantic conventions. (#6710)
- Added metrics support, and emit all stable metrics from the [Semantic Conventions](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-metrics.md) in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`. (#6747)
- Support [Go 1.24]. (#6765)
Expand Down
16 changes: 16 additions & 0 deletions config/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ go 1.22.0
require (
github.com/prometheus/client_golang v1.20.5
github.com/stretchr/testify v1.10.0
go.opentelemetry.io/contrib/propagators/autoprop v0.59.0
go.opentelemetry.io/contrib/propagators/aws v1.34.0
go.opentelemetry.io/contrib/propagators/b3 v1.34.0
go.opentelemetry.io/contrib/propagators/jaeger v1.34.0
go.opentelemetry.io/contrib/propagators/ot v1.34.0
go.opentelemetry.io/otel v1.34.0
go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.10.0
go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp v0.10.0
Expand Down Expand Up @@ -44,10 +49,21 @@ require (
go.opentelemetry.io/auto/sdk v1.1.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.34.0 // indirect
go.opentelemetry.io/proto/otlp v1.5.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/net v0.35.0 // indirect
golang.org/x/sys v0.30.0 // indirect
golang.org/x/text v0.22.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20250212204824-5a70512c5d8b // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20250212204824-5a70512c5d8b // indirect
google.golang.org/protobuf v1.36.5 // indirect
)

replace go.opentelemetry.io/contrib/propagators/autoprop => ../propagators/autoprop

replace go.opentelemetry.io/contrib/propagators/jaeger => ../propagators/jaeger

replace go.opentelemetry.io/contrib/propagators/b3 => ../propagators/b3

replace go.opentelemetry.io/contrib/propagators/aws => ../propagators/aws

replace go.opentelemetry.io/contrib/propagators/ot => ../propagators/ot
2 changes: 2 additions & 0 deletions config/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ go.opentelemetry.io/proto/otlp v1.5.0 h1:xJvq7gMzB31/d406fB8U5CBdyQGw4P399D1aQWU
go.opentelemetry.io/proto/otlp v1.5.0/go.mod h1:keN8WnHxOy8PG0rQZjJJ5A2ebUoafqWp0eVQ4yIXvJ4=
go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
golang.org/x/net v0.35.0 h1:T5GQRQb2y08kTAByq9L4/bz8cipCdA8FbRTXewonqY8=
golang.org/x/net v0.35.0/go.mod h1:EglIi67kWsHKlRzzVMUD93VMSWGFOMSZgxFjparz1Qk=
golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc=
Expand Down
14 changes: 14 additions & 0 deletions config/v0.3.0/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
nooplog "go.opentelemetry.io/otel/log/noop"
"go.opentelemetry.io/otel/metric"
noopmetric "go.opentelemetry.io/otel/metric/noop"
"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/trace"
nooptrace "go.opentelemetry.io/otel/trace/noop"
)
Expand Down Expand Up @@ -46,6 +47,7 @@
meterProvider metric.MeterProvider
tracerProvider trace.TracerProvider
loggerProvider log.LoggerProvider
propagator propagation.TextMapPropagator
shutdown shutdownFunc
}

Expand All @@ -64,6 +66,11 @@
return s.loggerProvider
}

// Propagator returns a configured propagation.TextMapPropagator.
func (s *SDK) Propagator() propagation.TextMapPropagator {
return s.propagator
}

// Shutdown calls shutdown on all configured providers.
func (s *SDK) Shutdown(ctx context.Context) error {
return s.shutdown(ctx)
Expand All @@ -73,6 +80,7 @@
loggerProvider: nooplog.LoggerProvider{},
meterProvider: noopmetric.MeterProvider{},
tracerProvider: nooptrace.TracerProvider{},
propagator: propagation.NewCompositeTextMapPropagator(),
shutdown: func(ctx context.Context) error { return nil },
}

Expand All @@ -86,6 +94,11 @@
return noopSDK, nil
}

propagator, err := propagator(o)
if err != nil {
return noopSDK, err
}

Check warning on line 100 in config/v0.3.0/config.go

View check run for this annotation

Codecov / codecov/patch

config/v0.3.0/config.go#L99-L100 

Added lines #L99 - L100 were not covered by tests

r := newResource(o.opentelemetryConfig.Resource)

mp, mpShutdown, err := meterProvider(o, r)
Expand All @@ -107,6 +120,7 @@
meterProvider: mp,
tracerProvider: tp,
loggerProvider: lp,
propagator: propagator,
shutdown: func(ctx context.Context) error {
return errors.Join(mpShutdown(ctx), tpShutdown(ctx), lpShutdown(ctx))
},
Expand Down
24 changes: 18 additions & 6 deletions config/v0.3.0/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/contrib/propagators/b3"
lognoop "go.opentelemetry.io/otel/log/noop"
metricnoop "go.opentelemetry.io/otel/metric/noop"
"go.opentelemetry.io/otel/propagation"
sdklog "go.opentelemetry.io/otel/sdk/log"
sdkmetric "go.opentelemetry.io/otel/sdk/metric"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
Expand All @@ -30,6 +32,7 @@ func TestNewSDK(t *testing.T) {
wantTracerProvider any
wantMeterProvider any
wantLoggerProvider any
wantPropagators any
wantErr error
wantShutdownErr error
}{
Expand All @@ -38,6 +41,7 @@ func TestNewSDK(t *testing.T) {
wantTracerProvider: tracenoop.NewTracerProvider(),
wantMeterProvider: metricnoop.NewMeterProvider(),
wantLoggerProvider: lognoop.NewLoggerProvider(),
wantPropagators: propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{}),
},
{
name: "with-configuration",
Expand All @@ -47,11 +51,15 @@ func TestNewSDK(t *testing.T) {
TracerProvider: &TracerProvider{},
MeterProvider: &MeterProvider{},
LoggerProvider: &LoggerProvider{},
Propagator: &Propagator{
Composite: []*string{ptr("b3")},
},
}),
},
wantTracerProvider: &sdktrace.TracerProvider{},
wantMeterProvider: &sdkmetric.MeterProvider{},
wantLoggerProvider: &sdklog.LoggerProvider{},
wantPropagators: b3.New(),
},
{
name: "with-sdk-disabled",
Expand All @@ -67,15 +75,19 @@ func TestNewSDK(t *testing.T) {
wantTracerProvider: tracenoop.NewTracerProvider(),
wantMeterProvider: metricnoop.NewMeterProvider(),
wantLoggerProvider: lognoop.NewLoggerProvider(),
wantPropagators: propagation.NewCompositeTextMapPropagator(),
},
}
for _, tt := range tests {
sdk, err := NewSDK(tt.cfg...)
require.Equal(t, tt.wantErr, err)
assert.IsType(t, tt.wantTracerProvider, sdk.TracerProvider())
assert.IsType(t, tt.wantMeterProvider, sdk.MeterProvider())
assert.IsType(t, tt.wantLoggerProvider, sdk.LoggerProvider())
require.Equal(t, tt.wantShutdownErr, sdk.Shutdown(context.Background()))
t.Run(tt.name, func(t *testing.T) {
sdk, err := NewSDK(tt.cfg...)
require.Equal(t, tt.wantErr, err)
assert.IsType(t, tt.wantTracerProvider, sdk.TracerProvider())
assert.IsType(t, tt.wantMeterProvider, sdk.MeterProvider())
assert.IsType(t, tt.wantLoggerProvider, sdk.LoggerProvider())
assert.Equal(t, tt.wantPropagators, sdk.Propagator())
require.Equal(t, tt.wantShutdownErr, sdk.Shutdown(context.Background()))
})
}
}

Expand Down
41 changes: 41 additions & 0 deletions config/v0.3.0/propagation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package config // import "go.opentelemetry.io/contrib/config/v0.3.0"

import (
"errors"

"go.opentelemetry.io/contrib/propagators/autoprop"
"go.opentelemetry.io/otel/propagation"
)

var (
errInvalidPropagatorEmpty = errors.New("invalid propagator name: empty")
errInvalidPropagatorNil = errors.New("invalid propagator name: nil")
)

func propagator(cfg configOptions) (propagation.TextMapPropagator, error) {
if cfg.opentelemetryConfig.Propagator == nil {
return autoprop.NewTextMapPropagator(), nil
}

n := len(cfg.opentelemetryConfig.Propagator.Composite)
if n == 0 {
return autoprop.NewTextMapPropagator(), nil
}
Comment on lines +23 to +26
Copy link
Member

Choose a reason for hiding this comment

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

There is a difference between

 "propagator": { 
 },

and

 "propagator": { 
     "composite": [] 
 },

From the specification:

If a property has a default value defined (i.e. is not required) and is missing or present but null, Create MUST ensure the SDK component is configured with the default value.

The first one should return the default value. The second an empty composite propagator.
The code should be probably like this:

	if cfg.opentelemetryConfig.Propagator.Composite == nil {
		return propagation.NewCompositeTextMapPropagator()
	}
	n := len(cfg.opentelemetryConfig.Propagator.Composite)
	if n == 0 {
		return autoprop.NewTextMapPropagator(), nil
	}


names := make([]string, 0, n)
for _, name := range cfg.opentelemetryConfig.Propagator.Composite {
if name == nil {
return nil, errInvalidPropagatorNil
}
if *name == "" {
return nil, errInvalidPropagatorEmpty
}

Check warning on line 35 in config/v0.3.0/propagation.go

View check run for this annotation

Codecov / codecov/patch

config/v0.3.0/propagation.go#L34-L35 

Added lines #L34 - L35 were not covered by tests
Comment on lines +33 to +35
Copy link
Member

@pellared pellared Feb 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I do not see anything in the schema that disallows using empty name. See: https://github.com/open-telemetry/opentelemetry-configuration/blob/main/schema/propagator.json

autoprop supports empty names (similarly to database/sql).

Sorry that I was not precise in #6727 (comment) 🙇

Copy link
Contributor

Choose a reason for hiding this comment

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

i opened an issue in the configuration repo to discuss this, i submitted a PR to propose enforcing a minimum length on propagator configuration


names = append(names, *name)
}

return autoprop.TextMapPropagator(names...)
}
Loading
Loading