Skip to content

Commit 3800104

Browse files
authored
Fix panic when no parent segment is found with AWS SDK v2 (#316)
To avoid triggering a panic when a parent segment is not present, we must call next middlewares instead of returning nil in the AWS SDK v2 middleware for X-Ray tracing. Fixes #315.
1 parent 53bbd9a commit 3800104

File tree

2 files changed

+96
-2
lines changed

2 files changed

+96
-2
lines changed

instrumentation/awsv2/awsv2.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func initializeMiddlewareAfter(stack *middleware.Stack) error {
2828
// Start the subsegment
2929
ctx, subseg := xray.BeginSubsegment(ctx, serviceName)
3030
if subseg == nil {
31-
return
31+
return next.HandleInitialize(ctx, in)
3232
}
3333
subseg.Namespace = "aws"
3434
subseg.GetAWS()["region"] = v2Middleware.GetRegion(ctx)
@@ -52,7 +52,11 @@ func deserializeMiddleware(stack *middleware.Stack) error {
5252
ctx context.Context, in middleware.DeserializeInput, next middleware.DeserializeHandler) (
5353
out middleware.DeserializeOutput, metadata middleware.Metadata, err error) {
5454

55-
subseg := ctx.Value(awsV2SubsegmentKey{}).(*xray.Segment)
55+
subseg, ok := ctx.Value(awsV2SubsegmentKey{}).(*xray.Segment)
56+
if !ok {
57+
return next.HandleDeserialize(ctx, in)
58+
}
59+
5660
in.Request.(*smithyhttp.Request).Header.Set(xray.TraceIDHeaderKey, subseg.DownstreamHeader().String())
5761

5862
out, metadata, err = next.HandleDeserialize(ctx, in)

instrumentation/awsv2/awsv2_test.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/aws/aws-sdk-go-v2/aws"
2222
"github.com/aws/aws-sdk-go-v2/service/route53"
2323
"github.com/aws/aws-sdk-go-v2/service/route53/types"
24+
"github.com/aws/aws-xray-sdk-go/strategy/ctxmissing"
2425
"github.com/aws/aws-xray-sdk-go/xray"
2526
)
2627

@@ -151,3 +152,92 @@ func TestAWSV2(t *testing.T) {
151152
time.Sleep(1 * time.Second)
152153
}
153154
}
155+
156+
func TestAWSV2WithoutSegment(t *testing.T) {
157+
cases := map[string]struct {
158+
responseStatus int
159+
responseBody []byte
160+
}{
161+
"fault response": {
162+
responseStatus: 500,
163+
responseBody: []byte(`<?xml version="1.0" encoding="UTF-8"?>
164+
<InvalidChangeBatch xmlns="https://route53.amazonaws.com/doc/2013-04-01/">
165+
<Messages>
166+
<Message>Tried to create resource record set duplicate.example.com. type A, but it already exists</Message>
167+
</Messages>
168+
<RequestId>b25f48e8-84fd-11e6-80d9-574e0c4664cb</RequestId>
169+
</InvalidChangeBatch>`),
170+
},
171+
172+
"error response": {
173+
responseStatus: 404,
174+
responseBody: []byte(`<?xml version="1.0"?>
175+
<ErrorResponse xmlns="http://route53.amazonaws.com/doc/2016-09-07/">
176+
<Error>
177+
<Type>Sender</Type>
178+
<Code>MalformedXML</Code>
179+
<Message>1 validation error detected: Value null at 'route53#ChangeSet' failed to satisfy constraint: Member must not be null</Message>
180+
</Error>
181+
<RequestId>1234567890A</RequestId>
182+
</ErrorResponse>
183+
`),
184+
},
185+
186+
"success response": {
187+
responseStatus: 200,
188+
responseBody: []byte(`<?xml version="1.0" encoding="UTF-8"?>
189+
<ChangeResourceRecordSetsResponse>
190+
<ChangeInfo>
191+
<Comment>mockComment</Comment>
192+
<Id>mockID</Id>
193+
</ChangeInfo>
194+
</ChangeResourceRecordSetsResponse>`),
195+
},
196+
}
197+
198+
for name, c := range cases {
199+
server := httptest.NewServer(http.HandlerFunc(
200+
func(w http.ResponseWriter, r *http.Request) {
201+
w.WriteHeader(c.responseStatus)
202+
_, err := w.Write(c.responseBody)
203+
if err != nil {
204+
t.Fatal(err)
205+
}
206+
}))
207+
defer server.Close()
208+
209+
t.Run(name, func(t *testing.T) {
210+
// Ignore errors when segment cannot be found.
211+
ctx, err := xray.ContextWithConfig(
212+
context.Background(),
213+
xray.Config{ContextMissingStrategy: ctxmissing.NewDefaultIgnoreErrorStrategy()},
214+
)
215+
if err != nil {
216+
t.Fatal(err)
217+
}
218+
219+
svc := route53.NewFromConfig(aws.Config{
220+
EndpointResolver: aws.EndpointResolverFunc(func(service, region string) (aws.Endpoint, error) {
221+
return aws.Endpoint{
222+
URL: server.URL,
223+
SigningName: "route53",
224+
}, nil
225+
}),
226+
Retryer: func() aws.Retryer {
227+
return aws.NopRetryer{}
228+
},
229+
})
230+
231+
_, _ = svc.ChangeResourceRecordSets(ctx, &route53.ChangeResourceRecordSetsInput{
232+
ChangeBatch: &types.ChangeBatch{
233+
Changes: []types.Change{},
234+
Comment: aws.String("mock"),
235+
},
236+
HostedZoneId: aws.String("zone"),
237+
}, func(options *route53.Options) {
238+
AWSV2Instrumentor(&options.APIOptions)
239+
})
240+
})
241+
time.Sleep(1 * time.Second)
242+
}
243+
}

0 commit comments

Comments
 (0)