From 4eb26b6d45b4add8fa834c0ff3034accc725dff4 Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Wed, 30 Apr 2025 13:22:04 +0200 Subject: [PATCH 1/6] feat(transport/http): Add ability to specify []otelhttp.Opts for transport when create new HTTP client --- internal/settings.go | 4 ++++ option/option.go | 15 +++++++++++++++ option/option_test.go | 27 +++++++++++++++++++++++++++ transport/http/dial.go | 2 +- 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/internal/settings.go b/internal/settings.go index beec4ea0ddb..95df0b3b2e3 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -15,6 +15,7 @@ import ( "time" "cloud.google.com/go/auth" + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "golang.org/x/oauth2" "golang.org/x/oauth2/google" "google.golang.org/api/internal/impersonate" @@ -76,6 +77,9 @@ type DialSettings struct { // TODO(b/372244283): Remove after b/358175516 has been fixed EnableAsyncRefreshDryRun func() + + // otelhttp options + OtelHTTPOpts []otelhttp.Option } // GetScopes returns the user-provided scopes, if set, or else falls back to the diff --git a/option/option.go b/option/option.go index 1b134caa862..061f7a0a82e 100644 --- a/option/option.go +++ b/option/option.go @@ -11,6 +11,7 @@ import ( "net/http" "cloud.google.com/go/auth" + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "golang.org/x/oauth2" "golang.org/x/oauth2/google" "google.golang.org/api/internal" @@ -414,3 +415,17 @@ type withLogger struct{ l *slog.Logger } func (w withLogger) Apply(o *internal.DialSettings) { o.Logger = w.l } + +// WithOtelHTTPOpts returns a ClientOption that sets the options for +// the OpenTelemetry HTTP client. This option is used to configure +// the OpenTelemetry HTTP client instrumentation. +func WithOtelHTTPOpts(opts ...otelhttp.Option) ClientOption { + return withOtelHTTPOpts{opts} +} + +type withOtelHTTPOpts struct{ opts []otelhttp.Option } + +func (w withOtelHTTPOpts) Apply(o *internal.DialSettings) { + o.OtelHTTPOpts = make([]otelhttp.Option, len(w.opts)) + copy(o.OtelHTTPOpts, w.opts) +} diff --git a/option/option_test.go b/option/option_test.go index 04c3716513b..e3316f97055 100644 --- a/option/option_test.go +++ b/option/option_test.go @@ -12,6 +12,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "golang.org/x/oauth2/google" "google.golang.org/api/internal" "google.golang.org/grpc" @@ -133,3 +134,29 @@ func TestApplyClientCertSource(t *testing.T) { t.Error(cmp.Diff(certGot, certWant, cmpopts.IgnoreUnexported(big.Int{}), cmpopts.IgnoreFields(tls.Certificate{}, "Leaf"))) } } + +func TestOtelHTTPOpts(t *testing.T) { + otelhttpopts := []otelhttp.Option{ + otelhttp.WithServerName("test"), + } + opts := []ClientOption{ + WithOtelHTTPOpts(otelhttpopts...), + } + var got internal.DialSettings + for _, opt := range opts { + opt.Apply(&got) + } + want := internal.DialSettings{ + OtelHTTPOpts: []otelhttp.Option{ + otelhttp.WithServerName("test"), + }, + } + + comparer := cmp.Comparer(func(x, y internal.DialSettings) bool { + return len(x.OtelHTTPOpts) == len(y.OtelHTTPOpts) + }) + + if !cmp.Equal(got, want, comparer) { + t.Error(cmp.Diff(got, want, comparer)) + } +} diff --git a/transport/http/dial.go b/transport/http/dial.go index a33df912035..0be3d8829d2 100644 --- a/transport/http/dial.go +++ b/transport/http/dial.go @@ -306,7 +306,7 @@ func addOpenTelemetryTransport(trans http.RoundTripper, settings *internal.DialS if settings.TelemetryDisabled { return trans } - return otelhttp.NewTransport(trans) + return otelhttp.NewTransport(trans, settings.OtelHTTPOpts...) } // clonedTransport returns the given RoundTripper as a cloned *http.Transport. From 50c53e813e960e031afb9039765291dc78918a6e Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Mon, 12 May 2025 17:43:29 +0200 Subject: [PATCH 2/6] feat(transport/all) Implement OpenTelemetryOpts as variadic to support otelhttp and otelgrpc --- internal/settings.go | 5 ++--- option/option.go | 20 ++++++++++---------- option/option_test.go | 21 +++++++-------------- transport/grpc/dial.go | 12 +++++++++--- transport/http/dial.go | 10 +++++++++- 5 files changed, 37 insertions(+), 31 deletions(-) diff --git a/internal/settings.go b/internal/settings.go index 95df0b3b2e3..b86894d8048 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -15,7 +15,6 @@ import ( "time" "cloud.google.com/go/auth" - "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "golang.org/x/oauth2" "golang.org/x/oauth2/google" "google.golang.org/api/internal/impersonate" @@ -78,8 +77,8 @@ type DialSettings struct { // TODO(b/372244283): Remove after b/358175516 has been fixed EnableAsyncRefreshDryRun func() - // otelhttp options - OtelHTTPOpts []otelhttp.Option + // otelhttp/otelgrpc options + OpenTelemetryOpts []any } // GetScopes returns the user-provided scopes, if set, or else falls back to the diff --git a/option/option.go b/option/option.go index 061f7a0a82e..d709f643506 100644 --- a/option/option.go +++ b/option/option.go @@ -11,7 +11,6 @@ import ( "net/http" "cloud.google.com/go/auth" - "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "golang.org/x/oauth2" "golang.org/x/oauth2/google" "google.golang.org/api/internal" @@ -416,16 +415,17 @@ func (w withLogger) Apply(o *internal.DialSettings) { o.Logger = w.l } -// WithOtelHTTPOpts returns a ClientOption that sets the options for -// the OpenTelemetry HTTP client. This option is used to configure -// the OpenTelemetry HTTP client instrumentation. -func WithOtelHTTPOpts(opts ...otelhttp.Option) ClientOption { - return withOtelHTTPOpts{opts} +// WithOpenTelemetryOpts returns a ClientOption that sets the options for +// the OpenTelemetry HTTP/gRPC client. This option is used to configure +// the OpenTelemetry HTTP/gRPC client instrumentation. +// It can accept any number of options, which can be otelhttp.Option or otelgrpc.Option. +func WithOpenTelemetryOpts(opts ...any) ClientOption { + return withOpenTelemetryOpts{opts} } -type withOtelHTTPOpts struct{ opts []otelhttp.Option } +type withOpenTelemetryOpts struct{ opts []any } -func (w withOtelHTTPOpts) Apply(o *internal.DialSettings) { - o.OtelHTTPOpts = make([]otelhttp.Option, len(w.opts)) - copy(o.OtelHTTPOpts, w.opts) +func (w withOpenTelemetryOpts) Apply(o *internal.DialSettings) { + o.OpenTelemetryOpts = make([]any, len(w.opts)) + copy(o.OpenTelemetryOpts, w.opts) } diff --git a/option/option_test.go b/option/option_test.go index e3316f97055..b4b6de90088 100644 --- a/option/option_test.go +++ b/option/option_test.go @@ -12,7 +12,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "golang.org/x/oauth2/google" "google.golang.org/api/internal" "google.golang.org/grpc" @@ -135,28 +134,22 @@ func TestApplyClientCertSource(t *testing.T) { } } -func TestOtelHTTPOpts(t *testing.T) { - otelhttpopts := []otelhttp.Option{ - otelhttp.WithServerName("test"), +func TestOpenTelemetryOpts(t *testing.T) { + otelOpts := []any{ + "non-otelhttp-otelgrpc-option", } opts := []ClientOption{ - WithOtelHTTPOpts(otelhttpopts...), + WithOpenTelemetryOpts(otelOpts...), } var got internal.DialSettings for _, opt := range opts { opt.Apply(&got) } want := internal.DialSettings{ - OtelHTTPOpts: []otelhttp.Option{ - otelhttp.WithServerName("test"), - }, + OpenTelemetryOpts: []any{}, } - comparer := cmp.Comparer(func(x, y internal.DialSettings) bool { - return len(x.OtelHTTPOpts) == len(y.OtelHTTPOpts) - }) - - if !cmp.Equal(got, want, comparer) { - t.Error(cmp.Diff(got, want, comparer)) + if cmp.Equal(got, want) { + t.Error(cmp.Diff(got, want)) } } diff --git a/transport/grpc/dial.go b/transport/grpc/dial.go index a6630a0e440..8b06595bbfe 100644 --- a/transport/grpc/dial.go +++ b/transport/grpc/dial.go @@ -68,9 +68,9 @@ var ( // otelGRPCStatsHandler returns singleton otelStatsHandler for reuse across all // dial connections. -func otelGRPCStatsHandler() stats.Handler { +func otelGRPCStatsHandler(opts []otelgrpc.Option) stats.Handler { initOtelStatsHandlerOnce.Do(func() { - otelStatsHandler = otelgrpc.NewClientHandler() + otelStatsHandler = otelgrpc.NewClientHandler(opts...) }) return otelStatsHandler } @@ -400,7 +400,13 @@ func addOpenTelemetryStatsHandler(opts []grpc.DialOption, settings *internal.Dia if settings.TelemetryDisabled { return opts } - return append(opts, grpc.WithStatsHandler(otelGRPCStatsHandler())) + otelOpts := []otelgrpc.Option{} + for _, opt := range settings.OpenTelemetryOpts { + if opt, ok := opt.(otelgrpc.Option); ok { + otelOpts = append(otelOpts, opt) + } + } + return append(opts, grpc.WithStatsHandler(otelGRPCStatsHandler(otelOpts))) } // grpcTokenSource supplies PerRPCCredentials from an oauth.TokenSource. diff --git a/transport/http/dial.go b/transport/http/dial.go index 0be3d8829d2..c290ecfa049 100644 --- a/transport/http/dial.go +++ b/transport/http/dial.go @@ -306,7 +306,15 @@ func addOpenTelemetryTransport(trans http.RoundTripper, settings *internal.DialS if settings.TelemetryDisabled { return trans } - return otelhttp.NewTransport(trans, settings.OtelHTTPOpts...) + + opts := []otelhttp.Option{} + for _, opt := range settings.OpenTelemetryOpts { + if opt, ok := opt.(otelhttp.Option); ok { + opts = append(opts, opt) + } + } + + return otelhttp.NewTransport(trans, opts...) } // clonedTransport returns the given RoundTripper as a cloned *http.Transport. From 16972eba7e285865338485e89ff8c7eeda69a245 Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Tue, 13 May 2025 15:54:36 +0200 Subject: [PATCH 3/6] (feature/all) Extend DialSettings.Validate to support OpenTelemetryOpts --- internal/settings.go | 16 +++++++++++++++- internal/settings_test.go | 11 +++++++++++ transport/grpc/dial.go | 7 +------ transport/http/dial.go | 10 +--------- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/internal/settings.go b/internal/settings.go index b86894d8048..b82370ecfe1 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -15,6 +15,8 @@ import ( "time" "cloud.google.com/go/auth" + "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "golang.org/x/oauth2" "golang.org/x/oauth2/google" "google.golang.org/api/internal/impersonate" @@ -78,7 +80,9 @@ type DialSettings struct { EnableAsyncRefreshDryRun func() // otelhttp/otelgrpc options - OpenTelemetryOpts []any + OpenTelemetryOpts []any + OpenTelemetryOptsGRPC []otelgrpc.Option + OpenTelemetryOptsHTTP []otelhttp.Option } // GetScopes returns the user-provided scopes, if set, or else falls back to the @@ -181,6 +185,16 @@ func (ds *DialSettings) Validate() error { if ds.ImpersonationConfig != nil && len(ds.ImpersonationConfig.Scopes) == 0 && len(ds.Scopes) == 0 { return errors.New("WithImpersonatedCredentials requires scopes being provided") } + for _, opt := range ds.OpenTelemetryOpts { + switch o := opt.(type) { + case otelhttp.Option: + ds.OpenTelemetryOptsHTTP = append(ds.OpenTelemetryOptsHTTP, o) + case otelgrpc.Option: + ds.OpenTelemetryOptsGRPC = append(ds.OpenTelemetryOptsGRPC, o.(otelgrpc.Option)) + default: + return errors.New("WithOpenTelemetryOpts options must be of type otelhttp.Option or otelgrpc.Option") + } + } return nil } diff --git a/internal/settings_test.go b/internal/settings_test.go index 09ccd2d4985..f1f39681304 100644 --- a/internal/settings_test.go +++ b/internal/settings_test.go @@ -12,7 +12,10 @@ import ( "google.golang.org/api/internal/impersonate" "google.golang.org/grpc" + "google.golang.org/grpc/stats" + "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "golang.org/x/oauth2" "golang.org/x/oauth2/google" ) @@ -38,6 +41,13 @@ func TestSettingsValidate(t *testing.T) { {ClientCertSource: dummyGetClientCertificate}, {ImpersonationConfig: &impersonate.Config{Scopes: []string{"x"}}}, {ImpersonationConfig: &impersonate.Config{}, Scopes: []string{"x"}}, + {OpenTelemetryOpts: []any{ + otelgrpc.WithFilter(func(ri *stats.RPCTagInfo) bool { + return true + }), + otelhttp.WithFilter(func(ri *http.Request) bool { + return true + })}}, } { err := ds.Validate() if err != nil { @@ -67,6 +77,7 @@ func TestSettingsValidate(t *testing.T) { {ClientCertSource: dummyGetClientCertificate, GRPCDialOpts: []grpc.DialOption{grpc.WithInsecure()}}, {ClientCertSource: dummyGetClientCertificate, GRPCConnPoolSize: 1}, {ImpersonationConfig: &impersonate.Config{}}, + {OpenTelemetryOpts: []any{"string"}}, } { err := ds.Validate() if err == nil { diff --git a/transport/grpc/dial.go b/transport/grpc/dial.go index 8b06595bbfe..35f7abd15f7 100644 --- a/transport/grpc/dial.go +++ b/transport/grpc/dial.go @@ -400,12 +400,7 @@ func addOpenTelemetryStatsHandler(opts []grpc.DialOption, settings *internal.Dia if settings.TelemetryDisabled { return opts } - otelOpts := []otelgrpc.Option{} - for _, opt := range settings.OpenTelemetryOpts { - if opt, ok := opt.(otelgrpc.Option); ok { - otelOpts = append(otelOpts, opt) - } - } + otelOpts := settings.OpenTelemetryOptsGRPC return append(opts, grpc.WithStatsHandler(otelGRPCStatsHandler(otelOpts))) } diff --git a/transport/http/dial.go b/transport/http/dial.go index c290ecfa049..ece8d3d2f2a 100644 --- a/transport/http/dial.go +++ b/transport/http/dial.go @@ -306,15 +306,7 @@ func addOpenTelemetryTransport(trans http.RoundTripper, settings *internal.DialS if settings.TelemetryDisabled { return trans } - - opts := []otelhttp.Option{} - for _, opt := range settings.OpenTelemetryOpts { - if opt, ok := opt.(otelhttp.Option); ok { - opts = append(opts, opt) - } - } - - return otelhttp.NewTransport(trans, opts...) + return otelhttp.NewTransport(trans, settings.OpenTelemetryOptsHTTP...) } // clonedTransport returns the given RoundTripper as a cloned *http.Transport. From 56370777d67903ee3a74ec8276b3ea4e739473a2 Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Tue, 13 May 2025 19:32:29 +0200 Subject: [PATCH 4/6] Specify full path --- internal/settings.go | 4 +++- option/option.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/settings.go b/internal/settings.go index b82370ecfe1..f65f04a00db 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -192,7 +192,9 @@ func (ds *DialSettings) Validate() error { case otelgrpc.Option: ds.OpenTelemetryOptsGRPC = append(ds.OpenTelemetryOptsGRPC, o.(otelgrpc.Option)) default: - return errors.New("WithOpenTelemetryOpts options must be of type otelhttp.Option or otelgrpc.Option") + return errors.New("WithOpenTelemetryOpts options must be of type " + + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp.Option " + + "or go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.Option") } } return nil diff --git a/option/option.go b/option/option.go index d709f643506..4ca24c366d8 100644 --- a/option/option.go +++ b/option/option.go @@ -418,7 +418,9 @@ func (w withLogger) Apply(o *internal.DialSettings) { // WithOpenTelemetryOpts returns a ClientOption that sets the options for // the OpenTelemetry HTTP/gRPC client. This option is used to configure // the OpenTelemetry HTTP/gRPC client instrumentation. -// It can accept any number of options, which can be otelhttp.Option or otelgrpc.Option. +// It can accept any number of options, which can be +// go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp.Option or +// go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.Option func WithOpenTelemetryOpts(opts ...any) ClientOption { return withOpenTelemetryOpts{opts} } From ae0f4b7ba8535dd01bd20e2dba64e4fc942def84 Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Wed, 14 May 2025 09:23:09 +0200 Subject: [PATCH 5/6] Fix type casting --- internal/settings.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/settings.go b/internal/settings.go index f65f04a00db..56711d1d913 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -190,7 +190,7 @@ func (ds *DialSettings) Validate() error { case otelhttp.Option: ds.OpenTelemetryOptsHTTP = append(ds.OpenTelemetryOptsHTTP, o) case otelgrpc.Option: - ds.OpenTelemetryOptsGRPC = append(ds.OpenTelemetryOptsGRPC, o.(otelgrpc.Option)) + ds.OpenTelemetryOptsGRPC = append(ds.OpenTelemetryOptsGRPC, o) default: return errors.New("WithOpenTelemetryOpts options must be of type " + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp.Option " + From 7803b4602b543f307216ea3e55da1df19e8cc290 Mon Sep 17 00:00:00 2001 From: Yury Gargay Date: Thu, 15 May 2025 16:55:36 +0200 Subject: [PATCH 6/6] Add TODO about go/auth --- transport/http/dial.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/transport/http/dial.go b/transport/http/dial.go index ece8d3d2f2a..3de2c16cc91 100644 --- a/transport/http/dial.go +++ b/transport/http/dial.go @@ -108,6 +108,8 @@ func newClientNewAuth(ctx context.Context, base http.RoundTripper, ds *internal. if ds.UserAgent != "" { headers.Set("User-Agent", ds.UserAgent) } + // TODO: propagate settings.OpenTelemetryOptsHTTP + // see https://github.com/googleapis/google-api-go-client/pull/3130#discussion_r2091318522 client, err := httptransport.NewClient(&httptransport.Options{ DisableTelemetry: ds.TelemetryDisabled, DisableAuthentication: ds.NoAuth,