From 5ade32f52a851bfd7cf90eabc329d65c148eb497 Mon Sep 17 00:00:00 2001 From: Luke Stoward Date: Tue, 11 Oct 2022 17:46:14 +0100 Subject: [PATCH 1/6] Add middleware for trace propagation --- .../aws/aws-sdk-go-v2/otelaws/aws.go | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go index 708d3232ec9..56826952d1a 100644 --- a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go +++ b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go @@ -16,6 +16,7 @@ package otelaws // import "go.opentelemetry.io/contrib/instrumentation/github.co import ( "context" + "fmt" "time" v2Middleware "github.com/aws/aws-sdk-go-v2/aws/middleware" @@ -25,6 +26,7 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/propagation" semconv "go.opentelemetry.io/otel/semconv/v1.12.0" "go.opentelemetry.io/otel/trace" ) @@ -110,6 +112,25 @@ func (m otelMiddlewares) deserializeMiddleware(stack *middleware.Stack) error { middleware.Before) } +func (m otelMiddlewares) finalizeMiddleware(stack *middleware.Stack) error { + return stack.Finalize.Add(middleware.FinalizeMiddlewareFunc("OTelFinalizeMiddleware", func( + ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler) ( + out middleware.FinalizeOutput, metadata middleware.Metadata, err error) { + + // Propagate the Trace information by injecting it into the HTTP request. + switch req := in.Request.(type) { + case *smithyhttp.Request: + otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(req.Header)) + default: + return out, metadata, fmt.Errorf("unknown transport type %T", req) + } + + return next.HandleFinalize(ctx, in) + + }), + middleware.Before) +} + // AppendMiddlewares attaches OTel middlewares to the AWS Go SDK V2 for instrumentation. // OTel middlewares can be appended to either all aws clients or a specific operation. // Please see more details in https://aws.github.io/aws-sdk-go-v2/docs/middleware/ @@ -128,5 +149,5 @@ func AppendMiddlewares(apiOptions *[]func(*middleware.Stack) error, opts ...Opti m := otelMiddlewares{tracer: cfg.TracerProvider.Tracer(tracerName, trace.WithInstrumentationVersion(SemVersion())), attributeSetter: cfg.AttributeSetter} - *apiOptions = append(*apiOptions, m.initializeMiddlewareBefore, m.initializeMiddlewareAfter, m.deserializeMiddleware) + *apiOptions = append(*apiOptions, m.initializeMiddlewareBefore, m.initializeMiddlewareAfter, m.finalizeMiddleware, m.deserializeMiddleware) } From 5dabd4c9f706d7d0e0e72c0819bbe1c4207c7f44 Mon Sep 17 00:00:00 2001 From: Luke Stoward Date: Tue, 11 Oct 2022 19:52:07 +0100 Subject: [PATCH 2/6] Add tests and make propagator configurable --- CHANGELOG.md | 5 ++ .../aws/aws-sdk-go-v2/otelaws/aws.go | 11 ++- .../aws/aws-sdk-go-v2/otelaws/aws_test.go | 79 +++++++++++++++++++ .../aws/aws-sdk-go-v2/otelaws/config.go | 16 +++- .../aws/aws-sdk-go-v2/otelaws/config_test.go | 32 ++++++++ 5 files changed, 135 insertions(+), 8 deletions(-) create mode 100644 instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws_test.go create mode 100644 instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 866d9dbb790..af04cce99e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +- Add trace propagation support to `instrumentation/github.com/aws/aws-sdk-go-v2/otelaws` (#2856). + ## [0.36.2] ### Changed @@ -20,6 +22,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `Inject` function in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` is deprecated. (#2838) - The `Extract` function in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` is deprecated. (#2838) +- google.golang.org/grpc/otelgrpc: Avoid getting a new Tracer for every RPC. +- google.golang.org/grpc/otelgrpc: Deprecate Inject and Extract public funcs. +- google.golang.org/grpc/otelgrpc: Conditionally compute message size for tracing events using proto v2 API rather than legacy v1 API. ## [0.36.1] diff --git a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go index 56826952d1a..c419d090e5a 100644 --- a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go +++ b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go @@ -16,7 +16,6 @@ package otelaws // import "go.opentelemetry.io/contrib/instrumentation/github.co import ( "context" - "fmt" "time" v2Middleware "github.com/aws/aws-sdk-go-v2/aws/middleware" @@ -42,6 +41,7 @@ type AttributeSetter func(context.Context, middleware.InitializeInput) []attribu type otelMiddlewares struct { tracer trace.Tracer + propagator propagation.TextMapPropagator attributeSetter []AttributeSetter } @@ -120,13 +120,10 @@ func (m otelMiddlewares) finalizeMiddleware(stack *middleware.Stack) error { // Propagate the Trace information by injecting it into the HTTP request. switch req := in.Request.(type) { case *smithyhttp.Request: - otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(req.Header)) - default: - return out, metadata, fmt.Errorf("unknown transport type %T", req) + m.propagator.Inject(ctx, propagation.HeaderCarrier(req.Header)) } return next.HandleFinalize(ctx, in) - }), middleware.Before) } @@ -136,7 +133,8 @@ func (m otelMiddlewares) finalizeMiddleware(stack *middleware.Stack) error { // Please see more details in https://aws.github.io/aws-sdk-go-v2/docs/middleware/ func AppendMiddlewares(apiOptions *[]func(*middleware.Stack) error, opts ...Option) { cfg := config{ - TracerProvider: otel.GetTracerProvider(), + TracerProvider: otel.GetTracerProvider(), + TextMapPropagator: otel.GetTextMapPropagator(), } for _, opt := range opts { opt.apply(&cfg) @@ -148,6 +146,7 @@ func AppendMiddlewares(apiOptions *[]func(*middleware.Stack) error, opts ...Opti m := otelMiddlewares{tracer: cfg.TracerProvider.Tracer(tracerName, trace.WithInstrumentationVersion(SemVersion())), + propagator: cfg.TextMapPropagator, attributeSetter: cfg.AttributeSetter} *apiOptions = append(*apiOptions, m.initializeMiddlewareBefore, m.initializeMiddlewareAfter, m.finalizeMiddleware, m.deserializeMiddleware) } diff --git a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws_test.go b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws_test.go new file mode 100644 index 00000000000..fd2fa3fd14d --- /dev/null +++ b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws_test.go @@ -0,0 +1,79 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package otelaws + +import ( + "context" + "net/http" + "testing" + + "github.com/aws/smithy-go/middleware" + smithyhttp "github.com/aws/smithy-go/transport/http" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/propagation" +) + +type mockPropagator struct { + injectKey string + injectValue string +} + +func (p mockPropagator) Inject(ctx context.Context, carrier propagation.TextMapCarrier) { + carrier.Set(p.injectKey, p.injectValue) +} +func (p mockPropagator) Extract(ctx context.Context, carrier propagation.TextMapCarrier) context.Context { + return context.TODO() +} +func (p mockPropagator) Fields() []string { + return []string{} +} + +func Test_otelMiddlewares_finalizeMiddleware(t *testing.T) { + stack := middleware.Stack{ + Finalize: middleware.NewFinalizeStep(), + } + + propagator := mockPropagator{ + injectKey: "mock-key", + injectValue: "mock-value", + } + + m := otelMiddlewares{ + propagator: propagator, + } + + err := m.finalizeMiddleware(&stack) + require.NoError(t, err) + + input := &smithyhttp.Request{ + Request: &http.Request{ + Header: http.Header{}, + }, + } + + next := middleware.HandlerFunc(func(ctx context.Context, input interface{}) (output interface{}, metadata middleware.Metadata, err error) { + return nil, middleware.Metadata{}, nil + }) + + stack.Finalize.HandleMiddleware(context.Background(), input, next) + + // Assert header has been updated with injected values + key := http.CanonicalHeaderKey(propagator.injectKey) + value := propagator.injectValue + + assert.Contains(t, input.Header, key) + assert.Contains(t, input.Header[key], value) +} diff --git a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config.go b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config.go index c5e8a4bc159..08ce1730373 100755 --- a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config.go +++ b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config.go @@ -15,12 +15,14 @@ package otelaws // import "go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws" import ( + "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" ) type config struct { - TracerProvider trace.TracerProvider - AttributeSetter []AttributeSetter + TracerProvider trace.TracerProvider + TextMapPropagator propagation.TextMapPropagator + AttributeSetter []AttributeSetter } // Option applies an option value. @@ -46,6 +48,16 @@ func WithTracerProvider(provider trace.TracerProvider) Option { }) } +// WithTextMapPropagator specifies a Text Map Propagator to use propagating context. +// If none is specified, the global TextMapPropagator is used. +func WithTextMapPropagator(propagator propagation.TextMapPropagator) Option { + return optionFunc(func(cfg *config) { + if propagator != nil { + cfg.TextMapPropagator = propagator + } + }) +} + // WithAttributeSetter specifies an attribute setter function for setting service specific attributes. // If none is specified, the service will be determined by the DefaultAttributeSetter function and the corresponding attributes will be included. func WithAttributeSetter(attributesetters ...AttributeSetter) Option { diff --git a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config_test.go b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config_test.go new file mode 100644 index 00000000000..e9e7f774cf0 --- /dev/null +++ b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config_test.go @@ -0,0 +1,32 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package otelaws + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel" +) + +func TestWithTextMapPropagator(t *testing.T) { + cfg := config{} + propagator := otel.GetTextMapPropagator() + + option := WithTextMapPropagator(propagator) + option.apply(&cfg) + + assert.Equal(t, cfg.TextMapPropagator, propagator) +} From 11bf28676ce945b96fe3d952bcca521f9f0e3453 Mon Sep 17 00:00:00 2001 From: Luke Stoward Date: Tue, 11 Oct 2022 19:55:15 +0100 Subject: [PATCH 3/6] Fix func doc --- instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config.go b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config.go index 08ce1730373..8ddc5f8a618 100755 --- a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config.go +++ b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config.go @@ -48,7 +48,7 @@ func WithTracerProvider(provider trace.TracerProvider) Option { }) } -// WithTextMapPropagator specifies a Text Map Propagator to use propagating context. +// WithTextMapPropagator specifies a Text Map Propagator to use when propagating context. // If none is specified, the global TextMapPropagator is used. func WithTextMapPropagator(propagator propagation.TextMapPropagator) Option { return optionFunc(func(cfg *config) { From 14e6efdda59114fa68c3fe7d63265334d7f24a0e Mon Sep 17 00:00:00 2001 From: Luke Stoward Date: Tue, 11 Oct 2022 22:02:49 +0100 Subject: [PATCH 4/6] Update changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af04cce99e5..01e81580a6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] -- Add trace propagation support to `instrumentation/github.com/aws/aws-sdk-go-v2/otelaws` (#2856). +### Added + +- Add trace context propagation support to `instrumentation/github.com/aws/aws-sdk-go-v2/otelaws` (#2856). ## [0.36.2] From b559c71ec624dc153617ce57c3dbe14b715c025e Mon Sep 17 00:00:00 2001 From: Luke Stoward Date: Wed, 12 Oct 2022 09:28:18 +0100 Subject: [PATCH 5/6] Fix linter errors --- instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go | 2 +- .../github.com/aws/aws-sdk-go-v2/otelaws/aws_test.go | 3 ++- .../github.com/aws/aws-sdk-go-v2/otelaws/config_test.go | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go index c419d090e5a..463ea24cafb 100644 --- a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go +++ b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws.go @@ -116,11 +116,11 @@ func (m otelMiddlewares) finalizeMiddleware(stack *middleware.Stack) error { return stack.Finalize.Add(middleware.FinalizeMiddlewareFunc("OTelFinalizeMiddleware", func( ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler) ( out middleware.FinalizeOutput, metadata middleware.Metadata, err error) { - // Propagate the Trace information by injecting it into the HTTP request. switch req := in.Request.(type) { case *smithyhttp.Request: m.propagator.Inject(ctx, propagation.HeaderCarrier(req.Header)) + default: } return next.HandleFinalize(ctx, in) diff --git a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws_test.go b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws_test.go index fd2fa3fd14d..c4df6c6cd5b 100644 --- a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws_test.go +++ b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/aws_test.go @@ -23,6 +23,7 @@ import ( smithyhttp "github.com/aws/smithy-go/transport/http" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/propagation" ) @@ -68,7 +69,7 @@ func Test_otelMiddlewares_finalizeMiddleware(t *testing.T) { return nil, middleware.Metadata{}, nil }) - stack.Finalize.HandleMiddleware(context.Background(), input, next) + _, _, _ = stack.Finalize.HandleMiddleware(context.Background(), input, next) // Assert header has been updated with injected values key := http.CanonicalHeaderKey(propagator.injectKey) diff --git a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config_test.go b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config_test.go index e9e7f774cf0..b60e75d6c45 100644 --- a/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config_test.go +++ b/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws/config_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel" ) From 205a5e12500c81c10115d6443759826a699971fd Mon Sep 17 00:00:00 2001 From: Luke Stoward Date: Wed, 12 Oct 2022 09:30:46 +0100 Subject: [PATCH 6/6] Fix changelog again --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01e81580a6c..98701c76608 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,9 +24,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `Inject` function in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` is deprecated. (#2838) - The `Extract` function in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` is deprecated. (#2838) -- google.golang.org/grpc/otelgrpc: Avoid getting a new Tracer for every RPC. -- google.golang.org/grpc/otelgrpc: Deprecate Inject and Extract public funcs. -- google.golang.org/grpc/otelgrpc: Conditionally compute message size for tracing events using proto v2 API rather than legacy v1 API. ## [0.36.1]