Skip to content

Commit

Permalink
Fix Active scope synchronization in AspNetScopeManager (#612)
Browse files Browse the repository at this point in the history
Issue:
When using the AspNetScopeManager, the closing of a span generated by automatic instrumentation doesn't get propagated correctly to the original ASP.NET synchronization context, causing later spans to have the wrong parent.

This occurs because we use ConfigureAwait(false) on all of our async continuations. When we create the new span we register it to the HttpContext.Current of the ASP.NET synchronization context, but when we close the span during the continuation we have lost the original ASP.NET synchronization context so the HttpContext.Current remains unchanged. Then, later spans generated in the ASP.NET synchronization context treat that span as being active and will always be a child of that one.

Solution:
When obtaining the Active scope, check the AsyncLocalCompat value first and then check HttpContext.Current as a fallback. This allows any changes to the Active scope that were made in our continuations without the ASP.NET synchronization context to be visible by later code run inside the ASP.NET synchronization context.

Additional fix:
Fix the System.Net.WebRequest.GetResponseAsync integration which had the wrong assembly name. In .NET Framework it is always in the System assembly. This was noticed while testing the modified sample. I am also adding the .NET Core integration because the change is so small.
  • Loading branch information
zacharycmontoya authored Jan 22, 2020
1 parent d969429 commit a9efb8a
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 6 deletions.
25 changes: 24 additions & 1 deletion integrations.json
Original file line number Diff line number Diff line change
Expand Up @@ -1853,7 +1853,30 @@
{
"caller": {},
"target": {
"assembly": "System.Net",
"assembly": "System",
"type": "System.Net.WebRequest",
"method": "GetResponseAsync",
"signature_types": [
"System.Threading.Tasks.Task`1<System.Net.WebResponse>"
],
"minimum_major": 4,
"minimum_minor": 0,
"minimum_patch": 0,
"maximum_major": 4,
"maximum_minor": 65535,
"maximum_patch": 65535
},
"wrapper": {
"assembly": "Datadog.Trace.ClrProfiler.Managed, Version=1.11.1.0, Culture=neutral, PublicKeyToken=def86d061d0d2eeb",
"type": "Datadog.Trace.ClrProfiler.Integrations.WebRequestIntegration",
"method": "GetResponseAsync",
"signature": "00 04 1C 1C 08 08 0A"
}
},
{
"caller": {},
"target": {
"assembly": "System.Net.Requests",
"type": "System.Net.WebRequest",
"method": "GetResponseAsync",
"signature_types": [
Expand Down
56 changes: 56 additions & 0 deletions samples-aspnet/Samples.AspNetMvc5_0/Controllers/HomeController.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Threading.Tasks;
using System.Web;
using System.Web.Mvc;
using Datadog.Trace;

namespace Samples.AspNetMvc5_0.Controllers
{
Expand Down Expand Up @@ -38,6 +44,56 @@ public ActionResult Contact()
return View();
}

public async Task<ActionResult> DadJoke()
{
string responseText;
var request = WebRequest.Create("https://icanhazdadjoke.com/");
((HttpWebRequest)request).Accept = "application/json;q=1";

using (var response = await request.GetResponseAsync())
using (var stream = response.GetResponseStream())
using (var reader = new StreamReader(stream))
{
try
{
responseText = reader.ReadToEnd();
}
catch (Exception)
{
responseText = "ENCOUNTERED AN ERROR WHEN READING RESPONSE.";
}
}

using (var scope = Tracer.Instance.StartActive("not-http-request-child-first"))
{
await Task.Delay(TimeSpan.FromMilliseconds(1_000));
}

request = WebRequest.Create("https://icanhazdadjoke.com/");
((HttpWebRequest)request).Accept = "application/json;q=1";

using (var response = await request.GetResponseAsync())
using (var stream = response.GetResponseStream())
using (var reader = new StreamReader(stream))
{
try
{
responseText = reader.ReadToEnd();
}
catch (Exception)
{
responseText = "ENCOUNTERED AN ERROR WHEN READING RESPONSE.";
}
}

using (var scope = Tracer.Instance.StartActive("not-http-request-child-second"))
{
await Task.Delay(TimeSpan.FromMilliseconds(1_000));
}

return Json(responseText, JsonRequestBehavior.AllowGet);
}

public ActionResult BadRequest()
{
throw new Exception("Oops, it broke.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@
<Content Include="Scripts\jquery-3.3.1.min.map" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\..\src\Datadog.Trace.AspNet\Datadog.Trace.AspNet.csproj">
<Project>{b34edbc7-c5fb-409d-8472-bc7469d6f2bd}</Project>
<Name>Datadog.Trace.AspNet</Name>
</ProjectReference>
<ProjectReference Include="..\..\src\Datadog.Trace.ClrProfiler.Managed\Datadog.Trace.ClrProfiler.Managed.csproj">
<Project>{85f35aaf-d102-4960-8b41-3bd9cbd0e77f}</Project>
<Name>Datadog.Trace.ClrProfiler.Managed</Name>
Expand Down
8 changes: 4 additions & 4 deletions src/Datadog.Trace.AspNet/AspNetScopeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,24 @@ public override Scope Active
{
get
{
var activeScope = HttpContext.Current?.Items[_name] as Scope;
var activeScope = _activeScopeFallback.Get();
if (activeScope != null)
{
return activeScope;
}

return _activeScopeFallback.Get();
return HttpContext.Current?.Items[_name] as Scope;
}

protected set
{
_activeScopeFallback.Set(value);

var httpContext = HttpContext.Current;
if (httpContext != null)
{
httpContext.Items[_name] = value;
}

_activeScopeFallback.Set(value);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,13 @@ public static object GetResponse(object webRequest, int opCode, int mdToken, lon
/// <param name="moduleVersionPtr">A pointer to the module version GUID.</param>
/// <returns>Returns the value returned by the inner method call.</returns>
[InterceptMethod(
TargetAssembly = "System.Net",
TargetAssembly = "System", // .NET Framework
TargetType = WebRequestTypeName,
TargetSignatureTypes = new[] { "System.Threading.Tasks.Task`1<System.Net.WebResponse>" },
TargetMinimumVersion = Major4,
TargetMaximumVersion = Major4)]
[InterceptMethod(
TargetAssembly = "System.Net.Requests", // .NET Core
TargetType = WebRequestTypeName,
TargetSignatureTypes = new[] { "System.Threading.Tasks.Task`1<System.Net.WebResponse>" },
TargetMinimumVersion = Major4,
Expand Down

0 comments on commit a9efb8a

Please sign in to comment.