Prevent X-Amzn-Trace-Id from Being Added More Than Once

[chrisoverzero]

Under some circumstances, the chain of calls through
DelegatingHandler instances can reach a particular handler more than
once. When that handler is HttpClientXRayTracingHandler, it produces
an X-Amzn-Trace-Id header that cannot be parsed because it is
multiple headers. (As represented in a string by separating them
with ", ".)

This patch checks whether X-Amzn-Trace-Id has already been added, and
skips adding another if it already has been.

Issue #, if available:

N/A

Description of changes:

Adds X-Amzn-Trace-Id at most once.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[yogiraj07]

Hi @chrisoverzero ,
Thank you for the PR. Can you provide with sample app/ steps to reproduce this issue ? If you see any exception, please provide stack trace.
Since DelegatingHandler is called multiple times, this means BeginSubsegment is overridden in each call for the same request.

At any point , latest header value should be put in the request header, so that, if a downstream service is instrumented with X-Ray, the Segment/Subsegment generated by the the downstream service, gets connected with the calling service.

Best,
Yogi

[chrisoverzero]

I don’t have a reducible case at the moment, I'm afraid. The scenario is that we have a delegating handler that handles 401 responses from web services by retrieving an authn token. The stack of delegating handlers reaches the X-Ray handler twice when we have a cache miss. If you really want a sample, I can synthesize one, but I think the problem is rather clear when considering how the HttpRequestHeaders.Add method works.

The header ends up with a value that looks something like this when stringified: "Root=xyzxyzxyz; Sampled=1, Root=xyzxyz; Sampled=1". When server-side X-Ray attempts to parse this header, it splits it on semicolon, then splits on equals, then crashes when calling ToDictionary because the key "Sampled" is duplicated. I've added a server-side stack trace to the end of the comment.

It sounds as though I got the fix completely backwards; is that what you're telling me? The desired semantics are that if the "X-Amzn-Trace-Id" header is already present, we want to replace it entirely. I can get another push tomorrow by noon Eastern.

Server-Side Stack Trace

Unknown error responding to request: ArgumentException:
System.ArgumentException: An item with the same key has already been added. Key: Sampled
at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer)
at Amazon.XRay.Recorder.Core.Internal.Entities.TraceHeader.TryParse(String rawHeader, TraceHeader& header)
at Amazon.XRay.Recorder.Handlers.AspNetCore.Internal.AWSXRayMiddleware.ProcessHTTPRequest(HttpContext context)
at Amazon.XRay.Recorder.Handlers.AspNetCore.Internal.AWSXRayMiddleware.Invoke(HttpContext context)
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
at Amazon.Lambda.AspNetCoreServer.APIGatewayProxyFunction.ProcessRequest(ILambdaContext lambdaContext, Context context, InvokeFeatures features, Boolean rethrowUnhandledError)

[yogiraj07]

Hi @chrisoverzero ,
Thank you for letting us know the use case. Your PR is valid and I have merged it.

[chrisoverzero]

Great! Thank you so much.