diff --git a/CHANGELOG.md b/CHANGELOG.md index bfcacfc5e5e..66eb1c5f1ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Added support for providing `endpoint`, `pollingIntervalMs` and `initialSamplingRate` using environment variable `OTEL_TRACES_SAMPLER_ARG` in `go.opentelemetry.io/contrib/samples/jaegerremote`. (#6310) +### Fixed + +- Fix broken AWS presigned URLs when using instrumentation in `go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws`. (#5975) + 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 5709db82cf4..42ea8ecab10 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 @@ -83,6 +83,23 @@ func (m otelMiddlewares) initializeMiddlewareAfter(stack *middleware.Stack) erro middleware.After) } +func (m otelMiddlewares) finalizeMiddlewareAfter(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) + }), + middleware.After) +} + func (m otelMiddlewares) deserializeMiddleware(stack *middleware.Stack) error { return stack.Deserialize.Add(middleware.DeserializeMiddlewareFunc("OTelDeserializeMiddleware", func( ctx context.Context, in middleware.DeserializeInput, next middleware.DeserializeHandler) ( @@ -108,23 +125,6 @@ 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: - m.propagator.Inject(ctx, propagation.HeaderCarrier(req.Header)) - default: - } - - return next.HandleFinalize(ctx, in) - }), - middleware.Before) -} - func spanName(serviceID, operation string) string { spanName := serviceID if operation != "" { @@ -155,5 +155,5 @@ func AppendMiddlewares(apiOptions *[]func(*middleware.Stack) error, opts ...Opti propagator: cfg.TextMapPropagator, attributeSetter: cfg.AttributeSetter, } - *apiOptions = append(*apiOptions, m.initializeMiddlewareBefore, m.initializeMiddlewareAfter, m.finalizeMiddleware, m.deserializeMiddleware) + *apiOptions = append(*apiOptions, m.initializeMiddlewareBefore, m.initializeMiddlewareAfter, m.finalizeMiddlewareAfter, 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 index 91c03aec27c..da44ef3d7d4 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 @@ -7,12 +7,16 @@ import ( "context" "net/http" "testing" + "time" "github.com/aws/smithy-go/middleware" smithyhttp "github.com/aws/smithy-go/transport/http" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/aws/aws-sdk-go-v2/aws" + awsSignerV4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" + "go.opentelemetry.io/otel/propagation" ) @@ -33,7 +37,7 @@ func (p mockPropagator) Fields() []string { return []string{} } -func Test_otelMiddlewares_finalizeMiddleware(t *testing.T) { +func Test_otelMiddlewares_finalizeMiddlewareAfter(t *testing.T) { stack := middleware.Stack{ Finalize: middleware.NewFinalizeStep(), } @@ -47,7 +51,7 @@ func Test_otelMiddlewares_finalizeMiddleware(t *testing.T) { propagator: propagator, } - err := m.finalizeMiddleware(&stack) + err := m.finalizeMiddlewareAfter(&stack) require.NoError(t, err) input := &smithyhttp.Request{ @@ -60,7 +64,8 @@ func Test_otelMiddlewares_finalizeMiddleware(t *testing.T) { return nil, middleware.Metadata{}, nil }) - _, _, _ = stack.Finalize.HandleMiddleware(context.Background(), input, next) + _, _, err = stack.Finalize.HandleMiddleware(context.Background(), input, next) + require.NoError(t, err) // Assert header has been updated with injected values key := http.CanonicalHeaderKey(propagator.injectKey) @@ -70,6 +75,77 @@ func Test_otelMiddlewares_finalizeMiddleware(t *testing.T) { assert.Contains(t, input.Header[key], value) } +type mockCredentialsProvider struct{} + +func (mockCredentialsProvider) Retrieve(context.Context) (aws.Credentials, error) { + return aws.Credentials{}, nil +} + +type mockHTTPPresigner struct{} + +func (f mockHTTPPresigner) PresignHTTP( + ctx context.Context, credentials aws.Credentials, r *http.Request, + payloadHash string, service string, region string, signingTime time.Time, + optFns ...func(*awsSignerV4.SignerOptions), +) ( + url string, signedHeader http.Header, err error, +) { + return "mock-url", nil, nil +} + +func Test_otelMiddlewares_presignedRequests(t *testing.T) { + stack := middleware.Stack{ + Finalize: middleware.NewFinalizeStep(), + } + + presignedHTTPMiddleware := awsSignerV4.NewPresignHTTPRequestMiddleware(awsSignerV4.PresignHTTPRequestMiddlewareOptions{ + CredentialsProvider: mockCredentialsProvider{}, + Presigner: mockHTTPPresigner{}, + LogSigning: false, + }) + + err := stack.Finalize.Add(presignedHTTPMiddleware, middleware.After) + require.NoError(t, err) + + propagator := mockPropagator{ + injectKey: "mock-key", + injectValue: "mock-value", + } + + m := otelMiddlewares{ + propagator: propagator, + } + + err = m.finalizeMiddlewareAfter(&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 + }) + + ctx := awsSignerV4.SetPayloadHash(context.Background(), "mock-hash") + url, _, err := stack.Finalize.HandleMiddleware(ctx, input, next) + + // verify we actually went through the presign flow + require.NoError(t, err) + presignedReq, ok := url.(*awsSignerV4.PresignedHTTPRequest) + require.True(t, ok) + require.Equal(t, "mock-url", presignedReq.URL) + + // Assert header has NOT been updated with injected values, as the presign middleware should short circuit + key := http.CanonicalHeaderKey(propagator.injectKey) + value := propagator.injectValue + + assert.NotContains(t, input.Header, key) + assert.NotContains(t, input.Header[key], value) +} + func Test_Span_name(t *testing.T) { serviceID1 := "" serviceID2 := "ServiceID"