From b66542529a143237a45bdcf2a11c09a8b1f7e655 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 13 May 2025 20:58:06 +0200 Subject: [PATCH] otlpmetrichttp: Add WithHTTPClient option (#6752) Follows https://github.com/open-telemetry/opentelemetry-go/pull/6688 Towards (for OTLP metric exporter): - https://github.com/open-telemetry/opentelemetry-go/issues/4536 - https://github.com/open-telemetry/opentelemetry-go/issues/5129 - https://github.com/open-telemetry/opentelemetry-go/issues/2632 --- CHANGELOG.md | 1 + .../otlpmetricgrpc/internal/oconf/options.go | 17 ++++++++--- .../internal/oconf/options_test.go | 18 ++++++++++++ .../otlp/otlpmetric/otlpmetrichttp/client.go | 29 ++++++++++--------- .../otlpmetric/otlpmetrichttp/client_test.go | 22 ++++++++++++++ .../otlp/otlpmetric/otlpmetrichttp/config.go | 16 ++++++++++ .../otlpmetrichttp/internal/oconf/options.go | 17 ++++++++--- .../internal/oconf/options_test.go | 18 ++++++++++++ .../otlp/otlpmetric/oconf/options.go.tmpl | 17 ++++++++--- .../otlpmetric/oconf/options_test.go.tmpl | 18 ++++++++++++ 10 files changed, 148 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d9befb28..9dfaecbdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm See the [migration documentation](./semconv/v1.31.0/MIGRATION.md) for information on how to upgrade from `go.opentelemetry.io/otel/semconv/v1.30.0`(#6479) - Add `Recording`, `Scope`, and `Record` types in `go.opentelemetry.io/otel/log/logtest`. (#6507) - Add `WithHTTPClient` option to configure the `http.Client` used by `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#6688) +- Add `WithHTTPClient` option to configure the `http.Client` used by `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#6752) ### Removed diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options.go index ba8e74a5c..cb77ae6a9 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options.go @@ -57,13 +57,15 @@ type ( Timeout time.Duration URLPath string - // gRPC configurations - GRPCCredentials credentials.TransportCredentials - TemporalitySelector metric.TemporalitySelector AggregationSelector metric.AggregationSelector - Proxy HTTPTransportProxyFunc + // gRPC configurations + GRPCCredentials credentials.TransportCredentials + + // HTTP configurations + Proxy HTTPTransportProxyFunc + HTTPClient *http.Client } Config struct { @@ -373,3 +375,10 @@ func WithProxy(pf HTTPTransportProxyFunc) GenericOption { return cfg }) } + +func WithHTTPClient(c *http.Client) GenericOption { + return newGenericOption(func(cfg Config) Config { + cfg.Metrics.HTTPClient = c + return cfg + }) +} diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go index 8bf1385bf..01369d4f5 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go @@ -533,6 +533,24 @@ func TestConfigs(t *testing.T) { assert.Nil(t, c.Metrics.Proxy) }, }, + + // HTTP Client Tests + { + name: "Test With HTTP Client", + opts: []GenericOption{ + WithHTTPClient(http.DefaultClient), + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, http.DefaultClient, c.Metrics.HTTPClient) + }, + }, + { + name: "Test Without HTTP Client", + opts: []GenericOption{}, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Nil(t, c.Metrics.HTTPClient) + }, + }, } for _, tt := range tests { diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go index aa61592a0..23f1f0031 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go @@ -55,20 +55,23 @@ var ourTransport = &http.Transport{ // newClient creates a new HTTP metric client. func newClient(cfg oconf.Config) (*client, error) { - httpClient := &http.Client{ - Transport: ourTransport, - Timeout: cfg.Metrics.Timeout, - } - - if cfg.Metrics.TLSCfg != nil || cfg.Metrics.Proxy != nil { - clonedTransport := ourTransport.Clone() - httpClient.Transport = clonedTransport - - if cfg.Metrics.TLSCfg != nil { - clonedTransport.TLSClientConfig = cfg.Metrics.TLSCfg + httpClient := cfg.Metrics.HTTPClient + if httpClient == nil { + httpClient = &http.Client{ + Transport: ourTransport, + Timeout: cfg.Metrics.Timeout, } - if cfg.Metrics.Proxy != nil { - clonedTransport.Proxy = cfg.Metrics.Proxy + + if cfg.Metrics.TLSCfg != nil || cfg.Metrics.Proxy != nil { + clonedTransport := ourTransport.Clone() + httpClient.Transport = clonedTransport + + if cfg.Metrics.TLSCfg != nil { + clonedTransport.TLSClientConfig = cfg.Metrics.TLSCfg + } + if cfg.Metrics.Proxy != nil { + clonedTransport.Proxy = cfg.Metrics.Proxy + } } } diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go index f600cc458..ef5c6774a 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go @@ -254,6 +254,28 @@ func TestConfig(t *testing.T) { }) t.Run("WithProxy", func(t *testing.T) { + headerKeySetInProxy := http.CanonicalHeaderKey("X-Using-Proxy") + headerValueSetInProxy := "true" + exp, coll := factoryFunc("", nil, WithHTTPClient(&http.Client{ + Transport: &http.Transport{ + Proxy: func(r *http.Request) (*url.URL, error) { + r.Header.Set(headerKeySetInProxy, headerValueSetInProxy) + return r.URL, nil + }, + }, + })) + ctx := context.Background() + t.Cleanup(func() { require.NoError(t, coll.Shutdown(ctx)) }) + require.NoError(t, exp.Export(ctx, &metricdata.ResourceMetrics{})) + // Ensure everything is flushed. + require.NoError(t, exp.Shutdown(ctx)) + + got := coll.Headers() + require.Contains(t, got, headerKeySetInProxy) + assert.Equal(t, []string{headerValueSetInProxy}, got[headerKeySetInProxy]) + }) + + t.Run("WithHTTPClient", func(t *testing.T) { headerKeySetInProxy := http.CanonicalHeaderKey("X-Using-Proxy") headerValueSetInProxy := "true" exp, coll := factoryFunc("", nil, WithProxy(func(r *http.Request) (*url.URL, error) { diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/config.go b/exporters/otlp/otlpmetric/otlpmetrichttp/config.go index bf05adcf1..2b144f7eb 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/config.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/config.go @@ -222,3 +222,19 @@ func WithAggregationSelector(selector metric.AggregationSelector) Option { func WithProxy(pf HTTPTransportProxyFunc) Option { return wrappedOption{oconf.WithProxy(oconf.HTTPTransportProxyFunc(pf))} } + +// WithHTTPClient sets the HTTP client to used by the exporter. +// +// This option will take precedence over [WithProxy], [WithTimeout], +// [WithTLSClientConfig] options as well as OTEL_EXPORTER_OTLP_CERTIFICATE, +// OTEL_EXPORTER_OTLP_METRICS_CERTIFICATE, OTEL_EXPORTER_OTLP_TIMEOUT, +// OTEL_EXPORTER_OTLP_METRICS_TIMEOUT environment variables. +// +// Timeout and all other fields of the passed [http.Client] are left intact. +// +// Be aware that passing an HTTP client with transport like +// [go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp.NewTransport] can +// cause the client to be instrumented twice and cause infinite recursion. +func WithHTTPClient(c *http.Client) Option { + return wrappedOption{oconf.WithHTTPClient(c)} +} diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options.go index 093853b1e..cfe629a97 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options.go @@ -57,13 +57,15 @@ type ( Timeout time.Duration URLPath string - // gRPC configurations - GRPCCredentials credentials.TransportCredentials - TemporalitySelector metric.TemporalitySelector AggregationSelector metric.AggregationSelector - Proxy HTTPTransportProxyFunc + // gRPC configurations + GRPCCredentials credentials.TransportCredentials + + // HTTP configurations + Proxy HTTPTransportProxyFunc + HTTPClient *http.Client } Config struct { @@ -373,3 +375,10 @@ func WithProxy(pf HTTPTransportProxyFunc) GenericOption { return cfg }) } + +func WithHTTPClient(c *http.Client) GenericOption { + return newGenericOption(func(cfg Config) Config { + cfg.Metrics.HTTPClient = c + return cfg + }) +} diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go index 34f320fb4..e3da8940e 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go @@ -533,6 +533,24 @@ func TestConfigs(t *testing.T) { assert.Nil(t, c.Metrics.Proxy) }, }, + + // HTTP Client Tests + { + name: "Test With HTTP Client", + opts: []GenericOption{ + WithHTTPClient(http.DefaultClient), + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, http.DefaultClient, c.Metrics.HTTPClient) + }, + }, + { + name: "Test Without HTTP Client", + opts: []GenericOption{}, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Nil(t, c.Metrics.HTTPClient) + }, + }, } for _, tt := range tests { diff --git a/internal/shared/otlp/otlpmetric/oconf/options.go.tmpl b/internal/shared/otlp/otlpmetric/oconf/options.go.tmpl index 3d30ba75e..6721aa567 100644 --- a/internal/shared/otlp/otlpmetric/oconf/options.go.tmpl +++ b/internal/shared/otlp/otlpmetric/oconf/options.go.tmpl @@ -57,13 +57,15 @@ type ( Timeout time.Duration URLPath string - // gRPC configurations - GRPCCredentials credentials.TransportCredentials - TemporalitySelector metric.TemporalitySelector AggregationSelector metric.AggregationSelector - Proxy HTTPTransportProxyFunc + // gRPC configurations + GRPCCredentials credentials.TransportCredentials + + // HTTP configurations + Proxy HTTPTransportProxyFunc + HTTPClient *http.Client } Config struct { @@ -373,3 +375,10 @@ func WithProxy(pf HTTPTransportProxyFunc) GenericOption { return cfg }) } + +func WithHTTPClient(c *http.Client) GenericOption { + return newGenericOption(func(cfg Config) Config { + cfg.Metrics.HTTPClient = c + return cfg + }) +} diff --git a/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl b/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl index 3ae7a58e3..22bc93fb7 100644 --- a/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl +++ b/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl @@ -533,6 +533,24 @@ func TestConfigs(t *testing.T) { assert.Nil(t, c.Metrics.Proxy) }, }, + + // HTTP Client Tests + { + name: "Test With HTTP Client", + opts: []GenericOption{ + WithHTTPClient(http.DefaultClient), + }, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Equal(t, http.DefaultClient, c.Metrics.HTTPClient) + }, + }, + { + name: "Test Without HTTP Client", + opts: []GenericOption{}, + asserts: func(t *testing.T, c *Config, grpcOption bool) { + assert.Nil(t, c.Metrics.HTTPClient) + }, + }, } for _, tt := range tests {