Skip to content

Commit 31dbd07

Browse files
ps-mirps-mir
authored
otlptracegrpc: fix grpc dialoption override bug (#8805)
User supplied `grpc.DialOptions` when supplied with `otlptracegrpc.WithDialOption` is being dropped because NewGRPCConfig applies its own internal defaults after applying user options. This led to internally computed entries land (and override) after user-supplied ones leading to drop of some settings like transport credentials. Resolves #2940 ### Fix Internal defaults are separated into its own slice (`dialOptsPrefix`). The final merging happens as ```go cfg.DialOptions = append(dialOptsPrefix, cfg.DialOptions...) ``` Which guarantees user supplied grpc options always take precedence, when used with `WithDialOption`. The change is made is shared `.tmpl`. The regenerated `NewGRPCConfig` is deadcode for `otlptracehttp`.
1 parent a22d2c0 commit 31dbd07

5 files changed

Lines changed: 96 additions & 21 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ The next release will require at least [Go 1.26].
6969
- Prevent a panic in `(*Set).Filter` when called on a nil receiver in `go.opentelemetry.io/otel/attribute`. (#8792)
7070
- The simple span and log processors record `otel.sdk.processor.{span,log}.processed` when the record is submitted to the exporter instead of after the export completes, and no longer set `error.type` from the export outcome, in `go.opentelemetry.io/otel/sdk/trace` and `go.opentelemetry.io/otel/sdk/log`. (#8705)
7171
- Prevent `Resource.MarshalLog` from panicking on nil resources in `go.opentelemetry.io/otel/sdk/resource`. (#8758)
72+
- Ensure `grpc.DialOption` values passed via `WithDialOption` take precedence over conflicting internally-computed defaults in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#8805)
7273

7374
## [1.45.0/0.67.0/0.21.0/0.0.18] - 2026-08-03
7475

‎exporters/otlp/otlptrace/otlptracegrpc/client_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,65 @@ func TestCustomUserAgent(t *testing.T) {
485485
require.Contains(t, headers.Get("user-agent")[0], customUserAgent)
486486
}
487487

488+
// A raw grpc.DialOption passed via WithDialOption must not be overridden by the
489+
// internally computed default credentials, which kick-in in absence of
490+
// otlptracegrpc.WithInsecure and otlptracegrpc.WithTLSCredentials.
491+
func TestWithDialOptionCredentialsTakePrecedence(t *testing.T) {
492+
mc := runMockCollector(t)
493+
t.Cleanup(func() { require.NoError(t, mc.stop()) })
494+
495+
ctx := context.Background() //nolint:usetesting // required to avoid getting a canceled context at cleanup.
496+
client := otlptracegrpc.NewClient(
497+
otlptracegrpc.WithEndpoint(mc.endpoint),
498+
otlptracegrpc.WithDialOption(grpc.WithTransportCredentials(insecure.NewCredentials())),
499+
)
500+
exp, err := otlptrace.New(ctx, client)
501+
require.NoError(t, err)
502+
t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
503+
504+
require.NoError(t, exp.ExportSpans(ctx, roSpans))
505+
}
506+
507+
// Guards against accidental replacement by an unrelated WithDialOption call:
508+
// it must not silently drop the exporter's default grpc.WithUserAgent dial option.
509+
func TestWithDialOptionPreservesDefaultUserAgent(t *testing.T) {
510+
mc := runMockCollector(t)
511+
t.Cleanup(func() { require.NoError(t, mc.stop()) })
512+
513+
ctx := context.Background() //nolint:usetesting // required to avoid getting a canceled context at cleanup.
514+
exp := newGRPCExporter(ctx, t, mc.endpoint,
515+
otlptracegrpc.WithDialOption(grpc.WithConnectParams(grpc.ConnectParams{
516+
Backoff: backoff.DefaultConfig,
517+
MinConnectTimeout: time.Second,
518+
})),
519+
)
520+
t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
521+
require.NoError(t, exp.ExportSpans(ctx, roSpans))
522+
523+
headers := mc.getHeaders()
524+
wantUserAgent := "OTel OTLP Exporter Go/" + otlptrace.Version()
525+
require.Contains(t, headers.Get("user-agent")[0], wantUserAgent)
526+
}
527+
528+
// Confirms WithDialOption's contract: repeated calls replace, not accumulate.
529+
func TestWithDialOptionLastCallWins(t *testing.T) {
530+
mc := runMockCollector(t)
531+
t.Cleanup(func() { require.NoError(t, mc.stop()) })
532+
533+
const firstUserAgent, secondUserAgent = "first-user-agent", "second-user-agent"
534+
ctx := context.Background() //nolint:usetesting // required to avoid getting a canceled context at cleanup.
535+
exp := newGRPCExporter(ctx, t, mc.endpoint,
536+
otlptracegrpc.WithDialOption(grpc.WithUserAgent(firstUserAgent)),
537+
otlptracegrpc.WithDialOption(grpc.WithUserAgent(secondUserAgent)),
538+
)
539+
t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
540+
require.NoError(t, exp.ExportSpans(ctx, roSpans))
541+
542+
headers := mc.getHeaders()
543+
require.Contains(t, headers.Get("user-agent")[0], secondUserAgent)
544+
require.NotContains(t, headers.Get("user-agent")[0], firstUserAgent)
545+
}
546+
488547
func TestClientInstrumentation(t *testing.T) {
489548
// Enable instrumentation for this test.
490549
t.Setenv("OTEL_GO_X_OBSERVABILITY", "true")

‎exporters/otlp/otlptrace/otlptracegrpc/internal/otlpconfig/options.go‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,37 +126,42 @@ func NewGRPCConfig(opts ...GRPCOption) Config {
126126
Timeout: DefaultTimeout,
127127
},
128128
RetryConfig: retry.DefaultConfig,
129-
DialOptions: []grpc.DialOption{grpc.WithUserAgent(userAgent)},
130129
}
131130
cfg = ApplyGRPCEnvConfigs(cfg)
132131
for _, opt := range opts {
133132
cfg = opt.ApplyGRPCOption(cfg)
134133
}
135134

135+
// dialOptsPrefix holds the internally computed defaults. It is prepended
136+
// to cfg.DialOptions so that a raw grpc.DialOption supplied via WithDialOption
137+
// always takes precedence: grpc.DialOption values are opaque closures, so this code has no way to
138+
// detect a conflicting user-supplied option and defer to it instead.
139+
dialOptsPrefix := []grpc.DialOption{grpc.WithUserAgent(userAgent)}
136140
if cfg.ServiceConfig != "" {
137-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
141+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
138142
}
139143
// Prioritize GRPCCredentials over Insecure (passing both is an error).
140144
if cfg.Traces.GRPCCredentials != nil { //nolint:gocritic // if-else is clearer than switch
141-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(cfg.Traces.GRPCCredentials))
145+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(cfg.Traces.GRPCCredentials))
142146
} else if cfg.Traces.Insecure {
143-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(insecure.NewCredentials()))
147+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(insecure.NewCredentials()))
144148
} else {
145149
// Default to using the host's root CA.
146150
creds := credentials.NewTLS(nil)
147151
cfg.Traces.GRPCCredentials = creds
148-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(creds))
152+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(creds))
149153
}
150154
if cfg.Traces.Compression == GzipCompression {
151-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
155+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
152156
}
153157
if cfg.ReconnectionPeriod != 0 {
154158
p := grpc.ConnectParams{
155159
Backoff: backoff.DefaultConfig,
156160
MinConnectTimeout: cfg.ReconnectionPeriod,
157161
}
158-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithConnectParams(p))
162+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithConnectParams(p))
159163
}
164+
cfg.DialOptions = append(dialOptsPrefix, cfg.DialOptions...)
160165

161166
return cfg
162167
}

‎exporters/otlp/otlptrace/otlptracehttp/internal/otlpconfig/options.go‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,37 +126,42 @@ func NewGRPCConfig(opts ...GRPCOption) Config {
126126
Timeout: DefaultTimeout,
127127
},
128128
RetryConfig: retry.DefaultConfig,
129-
DialOptions: []grpc.DialOption{grpc.WithUserAgent(userAgent)},
130129
}
131130
cfg = ApplyGRPCEnvConfigs(cfg)
132131
for _, opt := range opts {
133132
cfg = opt.ApplyGRPCOption(cfg)
134133
}
135134

135+
// dialOptsPrefix holds the internally computed defaults. It is prepended
136+
// to cfg.DialOptions so that a raw grpc.DialOption supplied via WithDialOption
137+
// always takes precedence: grpc.DialOption values are opaque closures, so this code has no way to
138+
// detect a conflicting user-supplied option and defer to it instead.
139+
dialOptsPrefix := []grpc.DialOption{grpc.WithUserAgent(userAgent)}
136140
if cfg.ServiceConfig != "" {
137-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
141+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
138142
}
139143
// Prioritize GRPCCredentials over Insecure (passing both is an error).
140144
if cfg.Traces.GRPCCredentials != nil { //nolint:gocritic // if-else is clearer than switch
141-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(cfg.Traces.GRPCCredentials))
145+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(cfg.Traces.GRPCCredentials))
142146
} else if cfg.Traces.Insecure {
143-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(insecure.NewCredentials()))
147+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(insecure.NewCredentials()))
144148
} else {
145149
// Default to using the host's root CA.
146150
creds := credentials.NewTLS(nil)
147151
cfg.Traces.GRPCCredentials = creds
148-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(creds))
152+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(creds))
149153
}
150154
if cfg.Traces.Compression == GzipCompression {
151-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
155+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
152156
}
153157
if cfg.ReconnectionPeriod != 0 {
154158
p := grpc.ConnectParams{
155159
Backoff: backoff.DefaultConfig,
156160
MinConnectTimeout: cfg.ReconnectionPeriod,
157161
}
158-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithConnectParams(p))
162+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithConnectParams(p))
159163
}
164+
cfg.DialOptions = append(dialOptsPrefix, cfg.DialOptions...)
160165

161166
return cfg
162167
}

‎internal/shared/otlp/otlptrace/otlpconfig/options.go.tmpl‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,37 +126,42 @@ func NewGRPCConfig(opts ...GRPCOption) Config {
126126
Timeout: DefaultTimeout,
127127
},
128128
RetryConfig: retry.DefaultConfig,
129-
DialOptions: []grpc.DialOption{grpc.WithUserAgent(userAgent)},
130129
}
131130
cfg = ApplyGRPCEnvConfigs(cfg)
132131
for _, opt := range opts {
133132
cfg = opt.ApplyGRPCOption(cfg)
134133
}
135134

135+
// dialOptsPrefix holds the internally computed defaults. It is prepended
136+
// to cfg.DialOptions so that a raw grpc.DialOption supplied via WithDialOption
137+
// always takes precedence: grpc.DialOption values are opaque closures, so this code has no way to
138+
// detect a conflicting user-supplied option and defer to it instead.
139+
dialOptsPrefix := []grpc.DialOption{grpc.WithUserAgent(userAgent)}
136140
if cfg.ServiceConfig != "" {
137-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
141+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
138142
}
139143
// Prioritize GRPCCredentials over Insecure (passing both is an error).
140144
if cfg.Traces.GRPCCredentials != nil { //nolint:gocritic // if-else is clearer than switch
141-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(cfg.Traces.GRPCCredentials))
145+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(cfg.Traces.GRPCCredentials))
142146
} else if cfg.Traces.Insecure {
143-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(insecure.NewCredentials()))
147+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(insecure.NewCredentials()))
144148
} else {
145149
// Default to using the host's root CA.
146150
creds := credentials.NewTLS(nil)
147151
cfg.Traces.GRPCCredentials = creds
148-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(creds))
152+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(creds))
149153
}
150154
if cfg.Traces.Compression == GzipCompression {
151-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
155+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
152156
}
153157
if cfg.ReconnectionPeriod != 0 {
154158
p := grpc.ConnectParams{
155159
Backoff: backoff.DefaultConfig,
156160
MinConnectTimeout: cfg.ReconnectionPeriod,
157161
}
158-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithConnectParams(p))
162+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithConnectParams(p))
159163
}
164+
cfg.DialOptions = append(dialOptsPrefix, cfg.DialOptions...)
160165

161166
return cfg
162167
}

0 commit comments

Comments
 (0)