Skip to content

Commit 267f72a

Browse files
ps-mirps-mir
authored
otlpmetricgrpc: fix preference for user supplied dialOptions (#8834)
User supplied grpc.DialOptions when supplied with otlpmetricgrpc.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. ## Fix This is the same bug and fix shape as #8805 (otlptracegrpc). One structural difference: otlpmetricgrpc never had `grpc.WithUserAgent` inside oconf's DialOptions to begin with (it's applied separately in client.go), so dialOptsPrefix here only needs to hold the service-config/credentials/compression/reconnect defaults. Fixes #8822 Related to #8805
1 parent 31dbd07 commit 267f72a

8 files changed

Lines changed: 154 additions & 19 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ See our [versioning policy](VERSIONING.md) for more information about these stab
3737
### Fixed
3838

3939
- Ignore attempts to unregister unknown span processors in `go.opentelemetry.io/otel/sdk/trace`. (#8840)
40+
- Ensure `grpc.DialOption` values passed via `WithDialOption` take precedence over conflicting internally-computed defaults in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc`. (#8834)
41+
- Ensure `grpc.DialOption` values passed via `WithDialOption` take precedence over conflicting internally-computed defaults in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#8805)
4042

4143
### Removed
4244

@@ -69,7 +71,6 @@ The next release will require at least [Go 1.26].
6971
- Prevent a panic in `(*Set).Filter` when called on a nil receiver in `go.opentelemetry.io/otel/attribute`. (#8792)
7072
- 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)
7173
- 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)
7374

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

‎exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"google.golang.org/genproto/googleapis/rpc/errdetails"
1414
"google.golang.org/grpc"
1515
"google.golang.org/grpc/codes"
16+
"google.golang.org/grpc/credentials/insecure"
1617
"google.golang.org/grpc/metadata"
1718
"google.golang.org/grpc/status"
1819
"google.golang.org/protobuf/types/known/durationpb"
@@ -253,6 +254,46 @@ func TestConfig(t *testing.T) {
253254
assert.Contains(t, got[key][0], customerUserAgent)
254255
})
255256

257+
t.Run("WithServiceConfig", func(t *testing.T) {
258+
exp, coll := factoryFunc(nil, WithServiceConfig("{}"))
259+
t.Cleanup(coll.Shutdown)
260+
ctx := t.Context()
261+
require.NoError(t, exp.Export(ctx, &metricdata.ResourceMetrics{}))
262+
require.NoError(t, exp.Shutdown(ctx))
263+
264+
assert.Len(t, coll.Collect().Dump(), 1)
265+
})
266+
267+
t.Run("WithReconnectionPeriod", func(t *testing.T) {
268+
exp, coll := factoryFunc(nil, WithReconnectionPeriod(50*time.Millisecond))
269+
t.Cleanup(coll.Shutdown)
270+
ctx := t.Context()
271+
require.NoError(t, exp.Export(ctx, &metricdata.ResourceMetrics{}))
272+
require.NoError(t, exp.Shutdown(ctx))
273+
274+
assert.Len(t, coll.Collect().Dump(), 1)
275+
})
276+
277+
// A raw grpc.DialOption passed via WithDialOption must not be overridden by the
278+
// internally computed default credentials, which kick in absent
279+
// WithInsecure and WithTLSCredentials.
280+
t.Run("WithDialOptionCredentialsTakePrecedence", func(t *testing.T) {
281+
coll, err := otest.NewGRPCCollector("", nil)
282+
require.NoError(t, err)
283+
t.Cleanup(coll.Shutdown)
284+
285+
ctx := context.Background() //nolint:usetesting // required to avoid getting a canceled context at cleanup.
286+
exp, err := New(ctx,
287+
WithEndpoint(coll.Addr().String()),
288+
WithDialOption(grpc.WithTransportCredentials(insecure.NewCredentials())),
289+
)
290+
require.NoError(t, err)
291+
t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
292+
293+
require.NoError(t, exp.Export(ctx, &metricdata.ResourceMetrics{}))
294+
assert.Len(t, coll.Collect().Dump(), 1)
295+
})
296+
256297
t.Run("WithMaxRequestSize", func(t *testing.T) {
257298
exp, coll := factoryFunc(
258299
nil,

‎exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,30 +144,36 @@ func NewGRPCConfig(opts ...GRPCOption) Config {
144144
cfg = opt.ApplyGRPCOption(cfg)
145145
}
146146

147+
// dialOptsPrefix holds the internally computed defaults. It is prepended
148+
// to cfg.DialOptions so that a raw grpc.DialOption supplied via WithDialOption
149+
// always takes precedence: grpc.DialOption values are opaque closures, so this code has no way to
150+
// detect a conflicting user-supplied option and defer to it instead.
151+
var dialOptsPrefix []grpc.DialOption
147152
if cfg.ServiceConfig != "" {
148-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
153+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
149154
}
150155
// Prioritize GRPCCredentials over Insecure (passing both is an error).
151156
if cfg.Metrics.GRPCCredentials != nil { //nolint:gocritic // if-else is clearer than switch
152-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(cfg.Metrics.GRPCCredentials))
157+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(cfg.Metrics.GRPCCredentials))
153158
} else if cfg.Metrics.Insecure {
154-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(insecure.NewCredentials()))
159+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(insecure.NewCredentials()))
155160
} else {
156161
// Default to using the host's root CA.
157162
creds := credentials.NewTLS(nil)
158163
cfg.Metrics.GRPCCredentials = creds
159-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(creds))
164+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(creds))
160165
}
161166
if cfg.Metrics.Compression == GzipCompression {
162-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
167+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
163168
}
164169
if cfg.ReconnectionPeriod != 0 {
165170
p := grpc.ConnectParams{
166171
Backoff: backoff.DefaultConfig,
167172
MinConnectTimeout: cfg.ReconnectionPeriod,
168173
}
169-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithConnectParams(p))
174+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithConnectParams(p))
170175
}
176+
cfg.DialOptions = append(dialOptsPrefix, cfg.DialOptions...)
171177

172178
return cfg
173179
}

‎exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"time"
1515

1616
"github.com/stretchr/testify/assert"
17+
"google.golang.org/grpc"
1718

1819
"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/envconfig"
1920
"go.opentelemetry.io/otel/sdk/metric"
@@ -561,6 +562,30 @@ func TestConfigs(t *testing.T) {
561562
assert.Nil(t, c.Metrics.HTTPClient)
562563
},
563564
},
565+
566+
{
567+
name: "Test With ServiceConfig, ReconnectionPeriod, And Caller-Supplied DialOption",
568+
opts: []GenericOption{
569+
newSplitOption(
570+
func(cfg Config) Config { return cfg },
571+
func(cfg Config) Config {
572+
cfg.ServiceConfig = "{}"
573+
cfg.ReconnectionPeriod = time.Second
574+
cfg.DialOptions = append(cfg.DialOptions, grpc.WithUserAgent("caller-supplied"))
575+
return cfg
576+
},
577+
),
578+
},
579+
asserts: func(t *testing.T, c *Config, grpcOption bool) { //nolint:revive // interface compliance
580+
if !grpcOption {
581+
return
582+
}
583+
baseline := NewGRPCConfig()
584+
// ServiceConfig, ReconnectionPeriod, and the caller-supplied DialOption
585+
// must each contribute their own entry alongside the internally computed defaults.
586+
assert.Len(t, c.DialOptions, len(baseline.DialOptions)+3)
587+
},
588+
},
564589
}
565590

566591
for _, tt := range tests {

‎exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,30 +144,36 @@ func NewGRPCConfig(opts ...GRPCOption) Config {
144144
cfg = opt.ApplyGRPCOption(cfg)
145145
}
146146

147+
// dialOptsPrefix holds the internally computed defaults. It is prepended
148+
// to cfg.DialOptions so that a raw grpc.DialOption supplied via WithDialOption
149+
// always takes precedence: grpc.DialOption values are opaque closures, so this code has no way to
150+
// detect a conflicting user-supplied option and defer to it instead.
151+
var dialOptsPrefix []grpc.DialOption
147152
if cfg.ServiceConfig != "" {
148-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
153+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
149154
}
150155
// Prioritize GRPCCredentials over Insecure (passing both is an error).
151156
if cfg.Metrics.GRPCCredentials != nil { //nolint:gocritic // if-else is clearer than switch
152-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(cfg.Metrics.GRPCCredentials))
157+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(cfg.Metrics.GRPCCredentials))
153158
} else if cfg.Metrics.Insecure {
154-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(insecure.NewCredentials()))
159+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(insecure.NewCredentials()))
155160
} else {
156161
// Default to using the host's root CA.
157162
creds := credentials.NewTLS(nil)
158163
cfg.Metrics.GRPCCredentials = creds
159-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(creds))
164+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(creds))
160165
}
161166
if cfg.Metrics.Compression == GzipCompression {
162-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
167+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
163168
}
164169
if cfg.ReconnectionPeriod != 0 {
165170
p := grpc.ConnectParams{
166171
Backoff: backoff.DefaultConfig,
167172
MinConnectTimeout: cfg.ReconnectionPeriod,
168173
}
169-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithConnectParams(p))
174+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithConnectParams(p))
170175
}
176+
cfg.DialOptions = append(dialOptsPrefix, cfg.DialOptions...)
171177

172178
return cfg
173179
}

‎exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"time"
1515

1616
"github.com/stretchr/testify/assert"
17+
"google.golang.org/grpc"
1718

1819
"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/envconfig"
1920
"go.opentelemetry.io/otel/sdk/metric"
@@ -561,6 +562,30 @@ func TestConfigs(t *testing.T) {
561562
assert.Nil(t, c.Metrics.HTTPClient)
562563
},
563564
},
565+
566+
{
567+
name: "Test With ServiceConfig, ReconnectionPeriod, And Caller-Supplied DialOption",
568+
opts: []GenericOption{
569+
newSplitOption(
570+
func(cfg Config) Config { return cfg },
571+
func(cfg Config) Config {
572+
cfg.ServiceConfig = "{}"
573+
cfg.ReconnectionPeriod = time.Second
574+
cfg.DialOptions = append(cfg.DialOptions, grpc.WithUserAgent("caller-supplied"))
575+
return cfg
576+
},
577+
),
578+
},
579+
asserts: func(t *testing.T, c *Config, grpcOption bool) { //nolint:revive // interface compliance
580+
if !grpcOption {
581+
return
582+
}
583+
baseline := NewGRPCConfig()
584+
// ServiceConfig, ReconnectionPeriod, and the caller-supplied DialOption
585+
// must each contribute their own entry alongside the internally computed defaults.
586+
assert.Len(t, c.DialOptions, len(baseline.DialOptions)+3)
587+
},
588+
},
564589
}
565590

566591
for _, tt := range tests {

‎internal/shared/otlp/otlpmetric/oconf/options.go.tmpl‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,30 +144,36 @@ func NewGRPCConfig(opts ...GRPCOption) Config {
144144
cfg = opt.ApplyGRPCOption(cfg)
145145
}
146146

147+
// dialOptsPrefix holds the internally computed defaults. It is prepended
148+
// to cfg.DialOptions so that a raw grpc.DialOption supplied via WithDialOption
149+
// always takes precedence: grpc.DialOption values are opaque closures, so this code has no way to
150+
// detect a conflicting user-supplied option and defer to it instead.
151+
var dialOptsPrefix []grpc.DialOption
147152
if cfg.ServiceConfig != "" {
148-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
153+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultServiceConfig(cfg.ServiceConfig))
149154
}
150155
// Prioritize GRPCCredentials over Insecure (passing both is an error).
151156
if cfg.Metrics.GRPCCredentials != nil { //nolint:gocritic // if-else is clearer than switch
152-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(cfg.Metrics.GRPCCredentials))
157+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(cfg.Metrics.GRPCCredentials))
153158
} else if cfg.Metrics.Insecure {
154-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(insecure.NewCredentials()))
159+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(insecure.NewCredentials()))
155160
} else {
156161
// Default to using the host's root CA.
157162
creds := credentials.NewTLS(nil)
158163
cfg.Metrics.GRPCCredentials = creds
159-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(creds))
164+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithTransportCredentials(creds))
160165
}
161166
if cfg.Metrics.Compression == GzipCompression {
162-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
167+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name)))
163168
}
164169
if cfg.ReconnectionPeriod != 0 {
165170
p := grpc.ConnectParams{
166171
Backoff: backoff.DefaultConfig,
167172
MinConnectTimeout: cfg.ReconnectionPeriod,
168173
}
169-
cfg.DialOptions = append(cfg.DialOptions, grpc.WithConnectParams(p))
174+
dialOptsPrefix = append(dialOptsPrefix, grpc.WithConnectParams(p))
170175
}
176+
cfg.DialOptions = append(dialOptsPrefix, cfg.DialOptions...)
171177

172178
return cfg
173179
}

‎internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"time"
1515

1616
"github.com/stretchr/testify/assert"
17+
"google.golang.org/grpc"
1718

1819
"{{ .envconfigImportPath }}"
1920
"go.opentelemetry.io/otel/sdk/metric"
@@ -561,6 +562,30 @@ func TestConfigs(t *testing.T) {
561562
assert.Nil(t, c.Metrics.HTTPClient)
562563
},
563564
},
565+
566+
{
567+
name: "Test With ServiceConfig, ReconnectionPeriod, And Caller-Supplied DialOption",
568+
opts: []GenericOption{
569+
newSplitOption(
570+
func(cfg Config) Config { return cfg },
571+
func(cfg Config) Config {
572+
cfg.ServiceConfig = "{}"
573+
cfg.ReconnectionPeriod = time.Second
574+
cfg.DialOptions = append(cfg.DialOptions, grpc.WithUserAgent("caller-supplied"))
575+
return cfg
576+
},
577+
),
578+
},
579+
asserts: func(t *testing.T, c *Config, grpcOption bool) { //nolint:revive // interface compliance
580+
if !grpcOption {
581+
return
582+
}
583+
baseline := NewGRPCConfig()
584+
// ServiceConfig, ReconnectionPeriod, and the caller-supplied DialOption
585+
// must each contribute their own entry alongside the internally computed defaults.
586+
assert.Len(t, c.DialOptions, len(baseline.DialOptions)+3)
587+
},
588+
},
564589
}
565590

566591
for _, tt := range tests {

0 commit comments

Comments
 (0)