diff --git a/CHANGELOG.md b/CHANGELOG.md index f8f6d207837..5cbe3acb9f7 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 ### Fixed +- Fix broken AWS presigned URLs when using instrumentation in `go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws`. (#5975) - Fixed the value for configuring the OTLP exporter to use `grpc` instead of `grpc/protobuf` in `go.opentelemetry.io/contrib/config`. (#6338) - Allow marshaling types in `go.opentelemetry.io/contrib/config`. (#6347) 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..ed64437990a 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,105 @@ func Test_otelMiddlewares_finalizeMiddleware(t *testing.T) { assert.Contains(t, input.Header[key], value) } +func Test_otelMiddlewares_finalizeMiddlewareAfter_Noop(t *testing.T) { + stack := middleware.Stack{ + Finalize: middleware.NewFinalizeStep(), + } + + propagator := mockPropagator{ + injectKey: "mock-key", + injectValue: "mock-value", + } + + m := otelMiddlewares{ + propagator: propagator, + } + + err := m.finalizeMiddlewareAfter(&stack) + require.NoError(t, err) + + // Non request input should trigger noop + input := &struct{}{} + + next := middleware.HandlerFunc(func(ctx context.Context, input interface{}) (output interface{}, metadata middleware.Metadata, err error) { + return nil, middleware.Metadata{}, nil + }) + + _, _, err = stack.Finalize.HandleMiddleware(context.Background(), input, next) + assert.NoError(t, err) +} + +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" diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/example/go.mod b/instrumentation/github.com/gin-gonic/gin/otelgin/example/go.mod index ac376495d46..b546a549e55 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/example/go.mod +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/example/go.mod @@ -17,7 +17,7 @@ require ( ) require ( - github.com/bytedance/sonic v1.12.4 // indirect + github.com/bytedance/sonic v1.12.5 // indirect github.com/bytedance/sonic/loader v0.2.1 // indirect github.com/cloudwego/base64x v0.1.4 // indirect github.com/cloudwego/iasm v0.2.0 // indirect diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/example/go.sum b/instrumentation/github.com/gin-gonic/gin/otelgin/example/go.sum index f45d89a681b..41087c5811e 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/example/go.sum +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/example/go.sum @@ -1,5 +1,5 @@ -github.com/bytedance/sonic v1.12.4 h1:9Csb3c9ZJhfUWeMtpCDCq6BUoH5ogfDFLUgQ/jG+R0k= -github.com/bytedance/sonic v1.12.4/go.mod h1:B8Gt/XvtZ3Fqj+iSKMypzymZxw/FVwgIGKzMzT9r/rk= +github.com/bytedance/sonic v1.12.5 h1:hoZxY8uW+mT+OpkcUWw4k0fDINtOcVavEsGfzwzFU/w= +github.com/bytedance/sonic v1.12.5/go.mod h1:B8Gt/XvtZ3Fqj+iSKMypzymZxw/FVwgIGKzMzT9r/rk= github.com/bytedance/sonic/loader v0.1.1/go.mod h1:ncP89zfokxS5LZrJxl5z0UJcsk4M4yY2JpfqGeCtNLU= github.com/bytedance/sonic/loader v0.2.1 h1:1GgorWTqf12TA8mma4DDSbaQigE2wOgQo7iCjjJv3+E= github.com/bytedance/sonic/loader v0.2.1/go.mod h1:ncP89zfokxS5LZrJxl5z0UJcsk4M4yY2JpfqGeCtNLU= diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod b/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod index a45e6216ccc..06efaeee2ac 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/go.mod @@ -13,7 +13,7 @@ require ( ) require ( - github.com/bytedance/sonic v1.12.4 // indirect + github.com/bytedance/sonic v1.12.5 // indirect github.com/bytedance/sonic/loader v0.2.1 // indirect github.com/cloudwego/base64x v0.1.4 // indirect github.com/cloudwego/iasm v0.2.0 // indirect diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum b/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum index 29db99e1171..df05f5d559c 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/go.sum @@ -1,5 +1,5 @@ -github.com/bytedance/sonic v1.12.4 h1:9Csb3c9ZJhfUWeMtpCDCq6BUoH5ogfDFLUgQ/jG+R0k= -github.com/bytedance/sonic v1.12.4/go.mod h1:B8Gt/XvtZ3Fqj+iSKMypzymZxw/FVwgIGKzMzT9r/rk= +github.com/bytedance/sonic v1.12.5 h1:hoZxY8uW+mT+OpkcUWw4k0fDINtOcVavEsGfzwzFU/w= +github.com/bytedance/sonic v1.12.5/go.mod h1:B8Gt/XvtZ3Fqj+iSKMypzymZxw/FVwgIGKzMzT9r/rk= github.com/bytedance/sonic/loader v0.1.1/go.mod h1:ncP89zfokxS5LZrJxl5z0UJcsk4M4yY2JpfqGeCtNLU= github.com/bytedance/sonic/loader v0.2.1 h1:1GgorWTqf12TA8mma4DDSbaQigE2wOgQo7iCjjJv3+E= github.com/bytedance/sonic/loader v0.2.1/go.mod h1:ncP89zfokxS5LZrJxl5z0UJcsk4M4yY2JpfqGeCtNLU= diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.mod b/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.mod index ee969a92ed7..28e56a0d15b 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.mod +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.mod @@ -12,7 +12,7 @@ require ( ) require ( - github.com/bytedance/sonic v1.12.4 // indirect + github.com/bytedance/sonic v1.12.5 // indirect github.com/bytedance/sonic/loader v0.2.1 // indirect github.com/cloudwego/base64x v0.1.4 // indirect github.com/cloudwego/iasm v0.2.0 // indirect diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.sum b/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.sum index dbe41cd1685..c861ebd2ce8 100644 --- a/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.sum +++ b/instrumentation/github.com/gin-gonic/gin/otelgin/test/go.sum @@ -1,5 +1,5 @@ -github.com/bytedance/sonic v1.12.4 h1:9Csb3c9ZJhfUWeMtpCDCq6BUoH5ogfDFLUgQ/jG+R0k= -github.com/bytedance/sonic v1.12.4/go.mod h1:B8Gt/XvtZ3Fqj+iSKMypzymZxw/FVwgIGKzMzT9r/rk= +github.com/bytedance/sonic v1.12.5 h1:hoZxY8uW+mT+OpkcUWw4k0fDINtOcVavEsGfzwzFU/w= +github.com/bytedance/sonic v1.12.5/go.mod h1:B8Gt/XvtZ3Fqj+iSKMypzymZxw/FVwgIGKzMzT9r/rk= github.com/bytedance/sonic/loader v0.1.1/go.mod h1:ncP89zfokxS5LZrJxl5z0UJcsk4M4yY2JpfqGeCtNLU= github.com/bytedance/sonic/loader v0.2.1 h1:1GgorWTqf12TA8mma4DDSbaQigE2wOgQo7iCjjJv3+E= github.com/bytedance/sonic/loader v0.2.1/go.mod h1:ncP89zfokxS5LZrJxl5z0UJcsk4M4yY2JpfqGeCtNLU=