aws / aws-xray-dotnet-agent

The official AWS X-Ray Auto Instrumentation Agent for .Net.
Apache License 2.0
23 stars 15 forks source link

Exception - An item with the same key has already been added #19

Open sdolier opened 4 years ago

sdolier commented 4 years ago

Hi,

We are intermittently receiving the following exception. It only occurs occasionally and seems to be after an application restart.

Our implementation of the agent is very basic and we do not have any custom segments set up.

Once this error occurs, every request to the site throws this exception until the application is restarted.

Exception information: 
    Exception type: ArgumentException 
    Exception message: An item with the same key has already been added.
   at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at System.Collections.Generic.Dictionary`2..ctor(IDictionary`2 dictionary, IEqualityComparer`1 comparer)
   at Amazon.XRay.Recorder.Core.AWSXRayRecorderImpl.AddRuleName(Segment newSegment, SamplingResponse sampleResponse)
   at Amazon.XRay.Recorder.Core.AWSXRayRecorderImpl.PopulateNewSegmentAttributes(Segment newSegment, SamplingResponse sampleResponse)
   at Amazon.XRay.Recorder.Core.AWSXRayRecorderImpl.BeginSegment(String name, String traceId, String parentId, SamplingResponse samplingResponse, Nullable`1 timestamp)
   at Amazon.XRay.Recorder.AutoInstrumentation.Utils.AspNetRequestUtil.ProcessHTTPRequest(Object sender, EventArgs e)
   at System.Web.HttpApplication.SyncEventExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute()
   at System.Web.HttpApplication.ExecuteStepImpl(IExecutionStep step)
   at System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously)
lupengamzn commented 4 years ago

Hi @sdolier ,

Thank you for your feedback,

It seems like this issue occurs when SDK is trying to add rule name to the segment.

I just did a little investigation and found out that, when adding (key,value) pairs to dictionary, you have to make sure that the same key should not already be in the dictionary, otherwise it will raise ArgumentException: An item with the same key has already been added. See link.

As .NET agent's using APIs from AWS X-Ray .NET SDK, I will make a PR there to fix it and will reference it after in the agent in the next release.

Thanks

sdolier commented 4 years ago

Thanks @lupengamzn

What looks odd to me is the stack trace includes a dictionary constructor.

at System.Collections.Generic.Dictionary`2..ctor(IDictionary`2 dictionary, IEqualityComparer`1 comparer)

Could it be this line that is causing an issue:

xrayContext = new Dictionary<string, string>(tempXrayContext); // deep copy for thread safety

The actual deep copy itself if the underlying contents of tempXrayContext is changing concurrently?

If that is the case, rather than creating a deep copy of newSegment.Aws["xray"], would it be better to change xrayContext to a ConcurrentDictionary?

eg, something like:

        private void AddRuleName(Segment newSegment, SamplingResponse sampleResponse)
        {
            var ruleName = sampleResponse.RuleName;
            string ruleNameKey = "sampling_rule_name";
            if (string.IsNullOrEmpty(ruleName))
            {
                return;
            }
            IDictionary<string, string> xrayContext;
            if (newSegment.Aws.TryGetValue("xray", out object value))
            {
                xrayContext = (ConcurrentDictionary<string, string>) value;
                xrayContext[ruleNameKey] = ruleName;
            }
            else
            {
                xrayContext = new ConcurrentDictionary<string, string>();
                xrayContext[ruleNameKey] = ruleName;
            }

            newSegment.Aws["xray"] = xrayContext;
        }
lupengamzn commented 4 years ago

Hey @sdolier ,

Sorry for the late reply as I was occupied with other tasks.

After investigation, this might be a thread issue when multiple threads trying to access the dictionary concurrently. Can you try changing Dictionary to ConcurrentDictionary as you mentioned in here? If this works and no similar exception is reported, please do submit a PR against the repo and we would love to review it.

As .NET agent is referencing AWSXRayRecorder.Core package from AWS X-Ray .NET SDK, if you want to make a change to .NET SDK package and test it on .NET agent, you can simply try git clone repo, modify code in AWSXRayRecorder.Core and have it referenced by .NET agent package(you may also need to remove agent's nuget dependency on AWSXRayRecorder.Core).

Thanks

lupengamzn commented 3 years ago

PR: https://github.com/aws/aws-xray-sdk-dotnet/pull/157 has been merged, X-Ray SDK will be released soon and the new SDK will be updated in the next release for .NET agent.