Address PR feedback

kevingosse · kevingosse · commit 352e75d040aa · 2021-05-28T14:37:28.000+02:00
diff --git a/src/Datadog.Trace.AspNet/TracingHttpModule.cs b/src/Datadog.Trace.AspNet/TracingHttpModule.cs
@@ -118,15 +118,15 @@ private void OnBeginRequest(object sender, EventArgs eventArgs)
 
                 tags.SetAnalyticsSampleRate(IntegrationId, tracer.Settings, enabledWithGlobalSetting: true);
 
-                httpContext.Items[_httpContextScopeKey] = scope;
-
                 // Decorate the incoming HTTP Request with distributed tracing headers
                 // in case the next processor cannot access the stored Scope
                 // (e.g. WCF being hosted in IIS)
                 if (HttpRuntime.UsingIntegratedPipeline)
                 {
                     SpanContextPropagator.Instance.Inject(scope.Span.Context, httpRequest.Headers.Wrap());
                 }
+
+                httpContext.Items[_httpContextScopeKey] = scope;
             }
             catch (Exception ex)
             {
@@ -149,28 +149,33 @@ private void OnEndRequest(object sender, EventArgs eventArgs)
                 if (sender is HttpApplication app &&
                     app.Context.Items[_httpContextScopeKey] is Scope scope)
                 {
-                    if (HttpRuntime.UsingIntegratedPipeline)
-                    {
-                        scope.Span.SetHeaderTags<IHeadersCollection>(app.Context.Response.Headers.Wrap(), Tracer.Instance.Settings.HeaderTags, defaultTagPrefix: SpanContextPropagator.HttpResponseHeadersTagPrefix);
-                    }
-
-                    scope.Span.SetHttpStatusCode(app.Context.Response.StatusCode, isServer: true);
-
-                    if (app.Context.Items[SharedConstants.HttpContextPropagatedResourceNameKey] is string resourceName
-                        && !string.IsNullOrEmpty(resourceName))
+                    try
                     {
-                        scope.Span.ResourceName = resourceName;
+                        if (HttpRuntime.UsingIntegratedPipeline)
+                        {
+                            scope.Span.SetHeaderTags<IHeadersCollection>(app.Context.Response.Headers.Wrap(), Tracer.Instance.Settings.HeaderTags, defaultTagPrefix: SpanContextPropagator.HttpResponseHeadersTagPrefix);
+                        }
+
+                        scope.Span.SetHttpStatusCode(app.Context.Response.StatusCode, isServer: true);
+
+                        if (app.Context.Items[SharedConstants.HttpContextPropagatedResourceNameKey] is string resourceName
+                            && !string.IsNullOrEmpty(resourceName))
+                        {
+                            scope.Span.ResourceName = resourceName;
+                        }
+                        else
+                        {
+                            string path = UriHelpers.GetCleanUriPath(app.Request.Url);
+                            scope.Span.ResourceName = $"{app.Request.HttpMethod.ToUpperInvariant()} {path.ToLowerInvariant()}";
+                        }
+
+                        scope.Dispose();
                     }
-                    else
+                    finally
                     {
-                        string path = UriHelpers.GetCleanUriPath(app.Request.Url);
-                        scope.Span.ResourceName = $"{app.Request.HttpMethod.ToUpperInvariant()} {path.ToLowerInvariant()}";
+                        // Clear the context to make sure another TracingHttpModule doesn't try to close the same scope
+                        TryClearContext(app.Context);
                     }
-
-                    scope.Dispose();
-
-                    // Clear the context to make sure another TracingHttpModule doesn't try to close the same scope
-                    app.Context.Items.Remove(_httpContextScopeKey);
                 }
             }
             catch (Exception ex)
@@ -208,5 +213,17 @@ private void OnError(object sender, EventArgs eventArgs)
                 Log.Error(ex, "Datadog ASP.NET HttpModule instrumentation error");
             }
         }
+
+        private void TryClearContext(HttpContext context)
+        {
+            try
+            {
+                context.Items.Remove(_httpContextScopeKey);
+            }
+            catch (Exception ex)
+            {
+                Log.Error(ex, "Error while clearing the HttpContext");
+            }
+        }
     }
 }
