Skip to content

Commit 3cafe9e

Browse files
pierotibouandrewlock
pierotibou
and
andrewlock
authored
Support child actions in aspnet (#2139)
* Add Child Action in tests to check it breaks * Handle Child actions use case Having a child action would overwrite the scope stored in items. We thus failed to close all spans leading to missing spans. ResourceName needs a special handling in that case to make sure we don't overwrite the resource name with the child action one. * Apply suggestions from code review Co-authored-by: Andrew Lock <[email protected]> * fix trailing whitespaces from github commit * RouteTemplateResourceNamesEnabled works with child actions * implement comments Co-authored-by: Andrew Lock <[email protected]>
1 parent 35c2d58 commit 3cafe9e

37 files changed

Lines changed: 690 additions & 86 deletions

File tree

‎tracer/src/Datadog.Trace/AspNet/SharedConstants.cs‎

Lines changed: 0 additions & 12 deletions
This file was deleted.

‎tracer/src/Datadog.Trace/AspNet/SharedItems.cs‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// <copyright file="SharedItems.cs" company="Datadog">
2+
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
4+
// </copyright>
5+
6+
#if NETFRAMEWORK
7+
8+
using System.Collections.Generic;
9+
using System.Web;
10+
11+
namespace Datadog.Trace.AspNet
12+
{
13+
internal static class SharedItems
14+
{
15+
public const string HttpContextPropagatedResourceNameKey = "__Datadog.Trace.ClrProfiler.Managed.AspNetMvcIntegration-aspnet.resourcename";
16+
17+
internal static void PushScope(HttpContext context, string key, Scope item)
18+
{
19+
if (context == null)
20+
{
21+
return;
22+
}
23+
24+
// Storing only the scope by default to avoid allocating a stack if no inner calls are done
25+
var existingItem = context.Items[key];
26+
if (existingItem is null)
27+
{
28+
context.Items[key] = item;
29+
}
30+
else if (existingItem is Stack<Scope> stack)
31+
{
32+
stack.Push(item);
33+
}
34+
else if (existingItem is Scope previousScope)
35+
{
36+
var newStack = new Stack<Scope>();
37+
newStack.Push(previousScope);
38+
newStack.Push(item);
39+
context.Items[key] = newStack;
40+
}
41+
}
42+
43+
internal static Scope TryPopScope(HttpContext context, string key)
44+
{
45+
var item = context?.Items[key];
46+
if (item is Scope storedScope)
47+
{
48+
return storedScope;
49+
}
50+
else if (item is Stack<Scope> stack && stack.Count > 0)
51+
{
52+
return stack.Pop();
53+
}
54+
55+
return default(Scope);
56+
}
57+
}
58+
}
59+
#endif

‎tracer/src/Datadog.Trace/AspNet/TracingHttpModule.cs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,11 @@
1111
using System.Linq;
1212
using System.Web;
1313
using Datadog.Trace.AppSec;
14-
using Datadog.Trace.AppSec.Transport.Http;
1514
using Datadog.Trace.Configuration;
1615
using Datadog.Trace.ExtensionMethods;
17-
using Datadog.Trace.Headers;
1816
using Datadog.Trace.Logging;
1917
using Datadog.Trace.Tagging;
2018
using Datadog.Trace.Util;
21-
using Datadog.Trace.Util.Http;
2219

2320
namespace Datadog.Trace.AspNet
2421
{
@@ -77,7 +74,6 @@ public void Dispose()
7774
private void OnBeginRequest(object sender, EventArgs eventArgs)
7875
{
7976
Scope scope = null;
80-
8177
try
8278
{
8379
var tracer = Tracer.Instance;
@@ -175,8 +171,8 @@ private void OnEndRequest(object sender, EventArgs eventArgs)
175171

176172
scope.Span.SetHttpStatusCode(app.Context.Response.StatusCode, isServer: true, Tracer.Instance.Settings);
177173

178-
if (app.Context.Items[SharedConstants.HttpContextPropagatedResourceNameKey] is string resourceName
179-
&& !string.IsNullOrEmpty(resourceName))
174+
if (app.Context.Items[SharedItems.HttpContextPropagatedResourceNameKey] is string resourceName
175+
&& !string.IsNullOrEmpty(resourceName))
180176
{
181177
scope.Span.ResourceName = resourceName;
182178
}

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/AspNetMvcIntegration.cs‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ internal static class AspNetMvcIntegration
2525
internal const string HttpContextKey = "__Datadog.Trace.ClrProfiler.Integrations.AspNetMvcIntegration";
2626

2727
private const string OperationName = "aspnet-mvc.request";
28+
private const string ChildActionOperationName = "aspnet-mvc.request.child-action";
2829

2930
private const string RouteCollectionRouteTypeName = "System.Web.Mvc.Routing.RouteCollectionRoute";
3031

@@ -43,7 +44,6 @@ internal static Scope CreateScope(ControllerContextStruct controllerContext)
4344
try
4445
{
4546
var httpContext = controllerContext.HttpContext;
46-
4747
if (httpContext == null)
4848
{
4949
return null;
@@ -63,6 +63,14 @@ internal static Scope CreateScope(ControllerContextStruct controllerContext)
6363
Route route = routeData?.Route as Route;
6464
RouteValueDictionary routeValues = routeData?.Values;
6565
bool wasAttributeRouted = false;
66+
bool isChildAction = controllerContext.ParentActionViewContext.RouteData?.Values["controller"] is not null;
67+
68+
if (isChildAction && newResourceNamesEnabled)
69+
{
70+
// For child actions, we want to stick to what was requested in the http request.
71+
// And the child action being a child, then we have already computed the resourcename.
72+
resourceName = httpContext.Items[SharedItems.HttpContextPropagatedResourceNameKey] as string;
73+
}
6674

6775
if (route == null && routeData?.Route.GetType().FullName == RouteCollectionRouteTypeName)
6876
{
@@ -155,7 +163,7 @@ internal static Scope CreateScope(ControllerContextStruct controllerContext)
155163
}
156164

157165
var tags = new AspNetTags();
158-
scope = Tracer.Instance.StartActiveInternal(OperationName, propagatedContext, tags: tags);
166+
scope = Tracer.Instance.StartActiveInternal(isChildAction ? ChildActionOperationName : OperationName, propagatedContext, tags: tags);
159167
span = scope.Span;
160168

161169
span.DecorateWebServerSpan(
@@ -173,10 +181,10 @@ internal static Scope CreateScope(ControllerContextStruct controllerContext)
173181

174182
tags.SetAnalyticsSampleRate(IntegrationId, tracer.Settings, enabledWithGlobalSetting: true);
175183

176-
if (newResourceNamesEnabled)
184+
if (newResourceNamesEnabled && string.IsNullOrEmpty(httpContext.Items[SharedItems.HttpContextPropagatedResourceNameKey] as string))
177185
{
178186
// set the resource name in the HttpContext so TracingHttpModule can update root span
179-
httpContext.Items[SharedConstants.HttpContextPropagatedResourceNameKey] = resourceName;
187+
httpContext.Items[SharedItems.HttpContextPropagatedResourceNameKey] = resourceName;
180188
}
181189
}
182190
}
@@ -189,5 +197,4 @@ internal static Scope CreateScope(ControllerContextStruct controllerContext)
189197
}
190198
}
191199
}
192-
193200
#endif

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/AspNetWebApi2Integration.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ internal static void UpdateSpan(IHttpControllerContext controllerContext, Span s
157157
var httpContext = System.Web.HttpContext.Current;
158158
if (httpContext is not null)
159159
{
160-
httpContext.Items[SharedConstants.HttpContextPropagatedResourceNameKey] = resourceName;
160+
httpContext.Items[SharedItems.HttpContextPropagatedResourceNameKey] = resourceName;
161161
}
162162
}
163163
}

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/AsyncControllerActionInvoker_BeginInvokeAction_Integration.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.ComponentModel;
99
using System.Web;
1010
using Datadog.Trace.AppSec;
11+
using Datadog.Trace.AspNet;
1112
using Datadog.Trace.ClrProfiler.CallTarget;
1213
using Datadog.Trace.Configuration;
1314
using Datadog.Trace.DuckTyping;
@@ -60,7 +61,7 @@ internal static CallTargetState OnMethodBegin<TTarget, TContext>(TTarget instanc
6061
{
6162
var duckedControllerContext = controllerContext.DuckCast<ControllerContextStruct>();
6263
scope = AspNetMvcIntegration.CreateScope(duckedControllerContext);
63-
HttpContext.Current.Items[AspNetMvcIntegration.HttpContextKey] = scope;
64+
SharedItems.PushScope(HttpContext.Current, AspNetMvcIntegration.HttpContextKey, scope);
6465

6566
var security = Security.Instance;
6667
if (security.Settings.Enabled)

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/AsyncControllerActionInvoker_EndInvokeAction_Integration.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System;
88
using System.ComponentModel;
99
using System.Web;
10+
using Datadog.Trace.AspNet;
1011
using Datadog.Trace.ClrProfiler.CallTarget;
1112
using Datadog.Trace.Configuration;
1213
using Datadog.Trace.ExtensionMethods;
@@ -54,7 +55,7 @@ internal static CallTargetReturn<TResult> OnMethodEnd<TTarget, TResult>(TTarget
5455

5556
try
5657
{
57-
scope = httpContext?.Items[AspNetMvcIntegration.HttpContextKey] as Scope;
58+
scope = SharedItems.TryPopScope(httpContext, AspNetMvcIntegration.HttpContextKey);
5859
}
5960
catch (Exception ex)
6061
{

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/ControllerContextStruct.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ internal struct ControllerContextStruct
2525
/// Gets the RouteData
2626
/// </summary>
2727
public RouteData RouteData;
28+
29+
/// <summary>
30+
/// Gets the ParentActionViewContext
31+
/// </summary>
32+
public ViewContextStruct ParentActionViewContext;
2833
}
2934
}
3035
#endif

‎tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/AspNet/ViewContextStruct.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// <copyright file="ViewContextStruct.cs" company="Datadog">
2+
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
4+
// </copyright>
5+
6+
#if NETFRAMEWORK
7+
using System.ComponentModel;
8+
using System.Web.Routing;
9+
using Datadog.Trace.DuckTyping;
10+
11+
namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.AspNet
12+
{
13+
/// <summary>
14+
/// ControllerContext struct copy target for ducktyping
15+
/// </summary>
16+
[DuckCopy]
17+
[Browsable(false)]
18+
[EditorBrowsable(EditorBrowsableState.Never)]
19+
public struct ViewContextStruct
20+
{
21+
/// <summary>
22+
/// Gets the RouteData
23+
/// </summary>
24+
public RouteData RouteData;
25+
}
26+
}
27+
#endif

‎tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/AspNet/AspNetMvc4Tests.cs‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -74,21 +74,21 @@ public AspNetMvc4Tests(IisFixture iisFixture, ITestOutputHelper output, bool cla
7474

7575
public static TheoryData<string, int> Data() => new()
7676
{
77-
{ "/Admin", 200 },
78-
{ "/Admin/Home", 200 },
79-
{ "/Admin/Home/Index", 200 },
80-
{ "/", 200 },
81-
{ "/Home", 200 },
82-
{ "/Home/Index", 200 },
83-
{ "/Home/BadRequest", 500 },
84-
{ "/Home/identifier", 500 },
85-
{ "/Home/identifier/123", 200 },
86-
{ "/Home/identifier/BadValue", 500 },
87-
{ "/Home/OptionalIdentifier", 200 },
88-
{ "/Home/OptionalIdentifier/123", 200 },
89-
{ "/Home/OptionalIdentifier/BadValue", 200 },
90-
{ "/Home/StatusCode?value=201", 201 },
91-
{ "/Home/StatusCode?value=503", 503 },
77+
{ "/Admin", 200 }, // Contains child actions
78+
{ "/Admin/Home", 200 }, // Contains child actions
79+
{ "/Admin/Home/Index", 200 }, // Contains child actions
80+
{ "/", 200 },
81+
{ "/Home", 200 },
82+
{ "/Home/Index", 200 },
83+
{ "/Home/BadRequest", 500 },
84+
{ "/Home/identifier", 500 },
85+
{ "/Home/identifier/123", 200 },
86+
{ "/Home/identifier/BadValue", 500 },
87+
{ "/Home/OptionalIdentifier", 200 },
88+
{ "/Home/OptionalIdentifier/123", 200 },
89+
{ "/Home/OptionalIdentifier/BadValue", 200 },
90+
{ "/Home/StatusCode?value=201", 201 },
91+
{ "/Home/StatusCode?value=503", 503 },
9292
};
9393

9494
[SkippableTheory]

0 commit comments

Comments
 (0)