Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Activity http.route and name #5026

Merged
merged 18 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@
using System.Runtime.CompilerServices;
#endif
using Microsoft.AspNetCore.Http;
#if NET6_0_OR_GREATER
#if !NETSTANDARD
using Microsoft.AspNetCore.Mvc.Diagnostics;
using Microsoft.AspNetCore.Mvc.RazorPages;
using Microsoft.AspNetCore.Routing;
#endif
using OpenTelemetry.Context.Propagation;
#if !NETSTANDARD2_0
Expand Down Expand Up @@ -202,7 +204,7 @@ public void OnStartActivity(Activity activity, object payload)
#endif

var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/";
activity.DisplayName = path;
activity.DisplayName = this.GetDisplayName(request.Method);

// see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/trace/semantic_conventions/http.md
if (this.emitOldAttributes)
Expand Down Expand Up @@ -252,15 +254,13 @@ public void OnStartActivity(Activity activity, object payload)
activity.SetTag(SemanticConventions.AttributeUrlQuery, request.QueryString.Value);
}

if (RequestMethodHelper.KnownMethods.TryGetValue(request.Method, out var httpMethod))
if (RequestMethodHelper.IsWellKnownHttpMethod(request.Method))
{
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, httpMethod);
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, request.Method);
}
else
{
// Set to default "_OTHER" as per spec.
// https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, "_OTHER");
activity.SetTag(SemanticConventions.AttributeHttpRequestMethod, RequestMethodHelper.OtherHttpMethod);
activity.SetTag(SemanticConventions.AttributeHttpRequestMethodOriginal, request.Method);
}

Expand Down Expand Up @@ -302,6 +302,19 @@ 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))
{
activity.DisplayName = this.GetDisplayName(context.Request.Method, routePattern);
activity.SetTag(SemanticConventions.AttributeHttpRoute, routePattern);
}
}
#endif

if (this.emitOldAttributes)
{
activity.SetTag(SemanticConventions.AttributeHttpStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode));
Expand Down Expand Up @@ -393,14 +406,11 @@ public void OnMvcBeforeAction(Activity activity, object payload)

if (activity.IsAllDataRequested)
{
#if !NET6_0_OR_GREATER
#if NETSTANDARD
_ = this.beforeActionActionDescriptorFetcher.TryFetch(payload, out var actionDescriptor);
_ = this.beforeActionAttributeRouteInfoFetcher.TryFetch(actionDescriptor, out var attributeRouteInfo);
_ = this.beforeActionTemplateFetcher.TryFetch(attributeRouteInfo, out var template);
#else
var beforeActionEventData = payload as BeforeActionEventData;
var template = beforeActionEventData.ActionDescriptor?.AttributeRouteInfo?.Template;
#endif

if (!string.IsNullOrEmpty(template))
{
// override the span name that was previously set to the path part of URL.
Expand All @@ -411,6 +421,22 @@ public void OnMvcBeforeAction(Activity activity, object payload)
// 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
}
}

Expand Down Expand Up @@ -509,7 +535,23 @@ private static bool TryGetGrpcMethod(Activity activity, out string grpcMethod)
grpcMethod = GrpcTagHelper.GetGrpcMethodFromActivity(activity);
return !string.IsNullOrEmpty(grpcMethod);
}
#endif

private string GetDisplayName(string httpMethod, string httpRoute = null)
{
var normalizedMethod = httpMethod;

if (this.emitNewAttributes)
{
normalizedMethod = RequestMethodHelper.IsWellKnownHttpMethod(httpMethod)
? httpMethod
: RequestMethodHelper.OtherHttpMethod;
}

return string.IsNullOrEmpty(httpRoute) ? normalizedMethod : $"{normalizedMethod} {httpRoute}";
}

#if !NETSTANDARD2_0
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext context)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,16 +200,12 @@ public void OnEventWritten_New(string name, object payload)
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol)));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeUrlScheme, context.Request.Scheme));
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(context.Response.StatusCode)));
if (RequestMethodHelper.KnownMethods.TryGetValue(context.Request.Method, out var httpMethod))
{
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, httpMethod));
}
else
{
// Set to default "_OTHER" as per spec.
// https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, "_OTHER"));
}

var httpMethod = context.Request.Method;
httpMethod = RequestMethodHelper.IsWellKnownHttpMethod(httpMethod)
? httpMethod
alanwest marked this conversation as resolved.
Show resolved Hide resolved
: RequestMethodHelper.OtherHttpMethod;
tags.Add(new KeyValuePair<string, object>(SemanticConventions.AttributeHttpRequestMethod, httpMethod));

#if NET6_0_OR_GREATER
var route = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,44 @@
// 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
9 changes: 9 additions & 0 deletions src/Shared/RequestMethodHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ namespace OpenTelemetry.Internal;

internal static class RequestMethodHelper
{
// The value "_OTHER" is used for non-standard HTTP methods.
// https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#common-attributes
public const string OtherHttpMethod = "_OTHER";

#if NET8_0_OR_GREATER
internal static readonly FrozenDictionary<string, string> KnownMethods;
#else
Expand Down Expand Up @@ -50,4 +54,9 @@ static RequestMethodHelper()
KnownMethods = knownMethodSet;
#endif
}

public static bool IsWellKnownHttpMethod(string method)
{
return KnownMethods.TryGetValue(method, out var _);
}
}
Loading