Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Remove subscription to MVC BeforeActionEvent
  • Loading branch information
alanwest committed Nov 14, 2023
commit 2ce8425902ee64cf555703efd69bb55dbb40cccb
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
#endif
using Microsoft.AspNetCore.Http;
#if !NETSTANDARD
using Microsoft.AspNetCore.Mvc.Diagnostics;
using Microsoft.AspNetCore.Mvc.RazorPages;
using Microsoft.AspNetCore.Routing;
#endif
using OpenTelemetry.Context.Propagation;
Expand All @@ -43,7 +41,6 @@ internal class HttpInListener : ListenerHandler
internal const string ActivityOperationName = "Microsoft.AspNetCore.Hosting.HttpRequestIn";
internal const string OnStartEvent = "Microsoft.AspNetCore.Hosting.HttpRequestIn.Start";
internal const string OnStopEvent = "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop";
internal const string OnMvcBeforeActionEvent = "Microsoft.AspNetCore.Mvc.BeforeAction";
internal const string OnUnhandledHostingExceptionEvent = "Microsoft.AspNetCore.Hosting.UnhandledException";
internal const string OnUnHandledDiagnosticsExceptionEvent = "Microsoft.AspNetCore.Diagnostics.UnhandledException";

Expand Down Expand Up @@ -99,12 +96,6 @@ public override void OnEventWritten(string name, object payload)
this.OnStopActivity(Activity.Current, payload);
}

break;
case OnMvcBeforeActionEvent:
{
this.OnMvcBeforeAction(Activity.Current, payload);
}

break;
case OnUnhandledHostingExceptionEvent:
case OnUnHandledDiagnosticsExceptionEvent:
Expand Down Expand Up @@ -295,15 +286,11 @@ public void OnStopActivity(Activity activity, object payload)
var response = context.Response;

#if !NETSTANDARD
var httpRoute = activity.GetTagValue(SemanticConventions.AttributeHttpRoute) as string;
if (string.IsNullOrEmpty(httpRoute))
var routePattern = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
if (!string.IsNullOrEmpty(routePattern))
{
var routePattern = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
if (!string.IsNullOrEmpty(routePattern))
{
activity.DisplayName = this.GetDisplayName(context.Request.Method, routePattern);
activity.SetTag(SemanticConventions.AttributeHttpRoute, routePattern);
}
activity.DisplayName = this.GetDisplayName(context.Request.Method, routePattern);
activity.SetTag(SemanticConventions.AttributeHttpRoute, routePattern);
}
#endif

Expand Down Expand Up @@ -368,70 +355,6 @@ public void OnStopActivity(Activity activity, object payload)
}
}

public void OnMvcBeforeAction(Activity activity, object payload)
{
// We cannot rely on Activity.Current here
// There could be activities started by middleware
// after activity started by framework resulting in different Activity.Current.
// so, we need to first find the activity started by Asp.Net Core.
// For .net6.0 onwards we could use IHttpActivityFeature to get the activity created by framework
// var httpActivityFeature = context.Features.Get<IHttpActivityFeature>();
// activity = httpActivityFeature.Activity;
// However, this will not work as in case of custom propagator
// we start a new activity during onStart event which is a sibling to the activity created by framework
// So, in that case we need to get the activity created by us here.
// we can do so only by looping through activity.Parent chain.
while (activity != null)
{
if (string.Equals(activity.OperationName, ActivityOperationName, StringComparison.Ordinal))
{
break;
}

activity = activity.Parent;
}

if (activity == null)
{
return;
}

if (activity.IsAllDataRequested)
{
#if NETSTANDARD
_ = this.beforeActionActionDescriptorFetcher.TryFetch(payload, out var actionDescriptor);
_ = this.beforeActionAttributeRouteInfoFetcher.TryFetch(actionDescriptor, out var attributeRouteInfo);
_ = this.beforeActionTemplateFetcher.TryFetch(attributeRouteInfo, out var template);

if (!string.IsNullOrEmpty(template))
{
// override the span name that was previously set to the path part of URL.
activity.DisplayName = template;
activity.SetTag(SemanticConventions.AttributeHttpRoute, template);
}

// TODO: Should we get values from RouteData?
// private readonly PropertyFetcher beforeActionRouteDataFetcher = new PropertyFetcher("routeData");
// var routeData = this.beforeActionRouteDataFetcher.Fetch(payload) as RouteData;
#else
var beforeActionEventData = payload as BeforeActionEventData;
var httpContext = beforeActionEventData.HttpContext;
var actionDescriptor = beforeActionEventData.ActionDescriptor;
var template = actionDescriptor?.AttributeRouteInfo?.Template;

// The template will be null when application does not use attribute routing
// For attribute routing, the DisplayName and http.route will be set when
// the activity ends.
if (actionDescriptor != null && (string.IsNullOrEmpty(template) || actionDescriptor is PageActionDescriptor))
{
var httpRoute = HttpTagHelper.GetHttpRouteFromActionDescriptor(httpContext, actionDescriptor);
activity.DisplayName = this.GetDisplayName(httpContext.Request.Method, httpRoute);
activity.SetTag(SemanticConventions.AttributeHttpRoute, httpRoute);
}
#endif
}
}

public void OnException(Activity activity, object payload)
{
if (activity.IsAllDataRequested)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,44 +14,13 @@
// limitations under the License.
// </copyright>

using System.Text.RegularExpressions;
#if !NETSTANDARD
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.Controllers;
using Microsoft.AspNetCore.Mvc.RazorPages;
using Microsoft.AspNetCore.Routing;
#endif

namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation;

/// <summary>
/// A collection of helper methods to be used when building Http activities.
/// </summary>
internal static class HttpTagHelper
{
private static readonly Regex ControllerNameRegex = new(@"\{controller=.*?\}+?", RegexOptions.Compiled);
private static readonly Regex ActionNameRegex = new(@"\{action=.*?\}+?", RegexOptions.Compiled);

#if !NETSTANDARD
public static string GetHttpRouteFromActionDescriptor(HttpContext httpContext, ActionDescriptor actionDescriptor)
{
var result = (httpContext.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;

if (actionDescriptor is ControllerActionDescriptor cad)
{
result = ControllerNameRegex.Replace(result, cad.ControllerName);
result = ActionNameRegex.Replace(result, cad.ActionName);
}
else if (actionDescriptor is PageActionDescriptor pad)
{
result = pad.ViewEnginePath;
}

return result;
}
#endif

/// <summary>
/// Gets the OpenTelemetry standard version tag value for a span based on its protocol/>.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -861,12 +861,6 @@ void ConfigureTestServices(IServiceCollection services)
numberofSubscribedEvents++;
}

break;
case HttpInListener.OnMvcBeforeActionEvent:
{
numberofSubscribedEvents++;
}

break;
default:
{
Expand Down Expand Up @@ -895,8 +889,8 @@ void ConfigureTestServices(IServiceCollection services)
using var response = await client.SendAsync(request).ConfigureAwait(false);
}

Assert.Equal(0, numberOfUnSubscribedEvents);
Assert.Equal(3, numberofSubscribedEvents);
Assert.Equal(1, numberOfUnSubscribedEvents);
Assert.Equal(2, numberofSubscribedEvents);
}

[Fact]
Expand Down Expand Up @@ -926,12 +920,6 @@ void ConfigureTestServices(IServiceCollection services)
numberofSubscribedEvents++;
}

break;
case HttpInListener.OnMvcBeforeActionEvent:
{
numberofSubscribedEvents++;
}

break;

// TODO: Add test case for validating name for both the types
Expand Down Expand Up @@ -979,8 +967,8 @@ void ConfigureTestServices(IServiceCollection services)
}

Assert.Equal(1, numberOfExceptionCallbacks);
Assert.Equal(0, numberOfUnSubscribedEvents);
Assert.Equal(4, numberofSubscribedEvents);
Assert.Equal(1, numberOfUnSubscribedEvents);
Assert.Equal(3, numberofSubscribedEvents);
}

[Fact(Skip = "https://github.com/open-telemetry/opentelemetry-dotnet/issues/4884")]
Expand Down
Loading