Skip to content

Commit

Permalink
Move all diagnostic source events to a single callback method `OnEven…
Browse files Browse the repository at this point in the history 
…tWritten` (#3691)
  • Loading branch information
@vishweshbankwar
vishweshbankwar authored Sep 26, 2022
1 parent ed0450e commit 8800b2f
Show file tree
Hide file tree
Showing 11 changed files with 272 additions and 292 deletions.
118 changes: 78 additions & 40 deletions src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,25 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
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";

#if NET7_0_OR_GREATER
// https://github.com/dotnet/aspnetcore/blob/8d6554e655b64da75b71e0e20d6db54a3ba8d2fb/src/Hosting/Hosting/src/GenericHost/GenericWebHostBuilder.cs#L85
internal static readonly string AspNetCoreActivitySourceName = "Microsoft.AspNetCore";
#endif

internal static readonly AssemblyName AssemblyName = typeof(HttpInListener).Assembly.GetName();
internal static readonly string ActivitySourceName = AssemblyName.Name;
internal static readonly Version Version = AssemblyName.Version;
internal static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version.ToString());

private const string DiagnosticSourceName = "Microsoft.AspNetCore";
private const string UnknownHostName = "UNKNOWN-HOST";

private static readonly Func<HttpRequest, string, IEnumerable<string>> HttpRequestHeaderValuesGetter = (request, name) => request.Headers[name];
private readonly PropertyFetcher<Exception> stopExceptionFetcher = new("Exception");
private readonly AspNetCoreInstrumentationOptions options;
Expand All @@ -54,8 +63,40 @@ public HttpInListener(AspNetCoreInstrumentationOptions options)
this.options = options;
}

public override void OnEventWritten(string name, object payload)
{
switch (name)
{
case OnStartEvent:
{
this.OnStartActivity(Activity.Current, payload);
}

break;
case OnStopEvent:
{
this.OnStopActivity(Activity.Current, payload);
}

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

break;
case OnUnhandledHostingExceptionEvent:
case OnUnHandledDiagnosticsExceptionEvent:
{
this.OnException(Activity.Current, payload);
}

break;
}
}

[System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "The objects should not be disposed.")]
public override void OnStartActivity(Activity activity, object payload)
public void OnStartActivity(Activity activity, object payload)
{
// The overall flow of what AspNetCore library does is as below:
// Activity.Start()
Expand Down Expand Up @@ -179,7 +220,7 @@ public override void OnStartActivity(Activity activity, object payload)
}
}

public override void OnStopActivity(Activity activity, object payload)
public void OnStopActivity(Activity activity, object payload)
{
if (activity.IsAllDataRequested)
{
Expand Down Expand Up @@ -239,55 +280,52 @@ public override void OnStopActivity(Activity activity, object payload)
}
}

public override void OnCustom(string name, Activity activity, object payload)
public void OnMvcBeforeAction(Activity activity, object payload)
{
if (name == "Microsoft.AspNetCore.Mvc.BeforeAction")
// 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)
{
// 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))
{
if (string.Equals(activity.OperationName, ActivityOperationName, StringComparison.Ordinal))
{
break;
}

activity = activity.Parent;
break;
}

if (activity == null)
{
return;
}
activity = activity.Parent;
}

if (activity.IsAllDataRequested)
{
var beforeActionEventData = payload as BeforeActionEventData;
var template = beforeActionEventData.ActionDescriptor?.AttributeRouteInfo?.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);
}
if (activity == null)
{
return;
}

// TODO: Should we get values from RouteData?
// private readonly PropertyFetcher beforeActionRouteDataFetcher = new PropertyFetcher("routeData");
// var routeData = this.beforeActionRouteDataFetcher.Fetch(payload) as RouteData;
if (activity.IsAllDataRequested)
{
var beforeActionEventData = payload as BeforeActionEventData;
var template = beforeActionEventData.ActionDescriptor?.AttributeRouteInfo?.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;
}
}

public override void OnException(Activity activity, object payload)
public void OnException(Activity activity, object payload)
{
if (activity.IsAllDataRequested)
{
Expand Down
101 changes: 53 additions & 48 deletions src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
{
internal class HttpInMetricsListener : ListenerHandler
{
private const string OnStopEvent = "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop";

private readonly Meter meter;
private readonly Histogram<double> httpServerDuration;

Expand All @@ -34,65 +36,68 @@ public HttpInMetricsListener(string name, Meter meter)
this.httpServerDuration = meter.CreateHistogram<double>("http.server.duration", "ms", "measures the duration of the inbound HTTP request");
}

public override void OnStopActivity(Activity activity, object payload)
public override void OnEventWritten(string name, object payload)
{
HttpContext context = payload as HttpContext;
if (context == null)
if (name == OnStopEvent)
{
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), nameof(this.OnStopActivity));
return;
}
HttpContext context = payload as HttpContext;
if (context == null)
{
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInMetricsListener), nameof(this.OnEventWritten));
return;
}

// TODO: Prometheus pulls metrics by invoking the /metrics endpoint. Decide if it makes sense to suppress this.
// Below is just a temporary way of achieving this suppression for metrics (we should consider suppressing traces too).
// If we want to suppress activity from Prometheus then we should use SuppressInstrumentationScope.
if (context.Request.Path.HasValue && context.Request.Path.Value.Contains("metrics"))
{
return;
}
// TODO: Prometheus pulls metrics by invoking the /metrics endpoint. Decide if it makes sense to suppress this.
// Below is just a temporary way of achieving this suppression for metrics (we should consider suppressing traces too).
// If we want to suppress activity from Prometheus then we should use SuppressInstrumentationScope.
if (context.Request.Path.HasValue && context.Request.Path.Value.Contains("metrics"))
{
return;
}

string host;
string host;

if (context.Request.Host.Port is null or 80 or 443)
{
host = context.Request.Host.Host;
}
else
{
host = context.Request.Host.Host + ":" + context.Request.Host.Port;
}
if (context.Request.Host.Port is null or 80 or 443)
{
host = context.Request.Host.Host;
}
else
{
host = context.Request.Host.Host + ":" + context.Request.Host.Port;
}

TagList tags;
TagList tags;

var target = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
var target = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;

// TODO: This is just a minimal set of attributes. See the spec for additional attributes:
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#http-server
if (!string.IsNullOrEmpty(target))
{
tags = new TagList
// TODO: This is just a minimal set of attributes. See the spec for additional attributes:
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#http-server
if (!string.IsNullOrEmpty(target))
{
{ SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol) },
{ SemanticConventions.AttributeHttpScheme, context.Request.Scheme },
{ SemanticConventions.AttributeHttpMethod, context.Request.Method },
{ SemanticConventions.AttributeHttpHost, host },
{ SemanticConventions.AttributeHttpTarget, target },
{ SemanticConventions.AttributeHttpStatusCode, context.Response.StatusCode.ToString() },
};
}
else
{
tags = new TagList
tags = new TagList
{
{ SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol) },
{ SemanticConventions.AttributeHttpScheme, context.Request.Scheme },
{ SemanticConventions.AttributeHttpMethod, context.Request.Method },
{ SemanticConventions.AttributeHttpHost, host },
{ SemanticConventions.AttributeHttpTarget, target },
{ SemanticConventions.AttributeHttpStatusCode, context.Response.StatusCode.ToString() },
};
}
else
{
{ SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol) },
{ SemanticConventions.AttributeHttpScheme, context.Request.Scheme },
{ SemanticConventions.AttributeHttpMethod, context.Request.Method },
{ SemanticConventions.AttributeHttpHost, host },
{ SemanticConventions.AttributeHttpStatusCode, context.Response.StatusCode.ToString() },
};
}
tags = new TagList
{
{ SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocol(context.Request.Protocol) },
{ SemanticConventions.AttributeHttpScheme, context.Request.Scheme },
{ SemanticConventions.AttributeHttpMethod, context.Request.Method },
{ SemanticConventions.AttributeHttpHost, host },
{ SemanticConventions.AttributeHttpStatusCode, context.Response.StatusCode.ToString() },
};
}

this.httpServerDuration.Record(activity.Duration.TotalMilliseconds, tags);
this.httpServerDuration.Record(Activity.Current.Duration.TotalMilliseconds, tags);
}
}
}
}
26 changes: 24 additions & 2 deletions ...penTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ internal sealed class GrpcClientDiagnosticListener : ListenerHandler
internal static readonly Version Version = AssemblyName.Version;
internal static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version.ToString());

private const string OnStartEvent = "Grpc.Net.Client.GrpcOut.Start";
private const string OnStopEvent = "Grpc.Net.Client.GrpcOut.Stop";

private readonly GrpcClientInstrumentationOptions options;
private readonly PropertyFetcher<HttpRequestMessage> startRequestFetcher = new("Request");
private readonly PropertyFetcher<HttpResponseMessage> stopRequestFetcher = new("Response");
Expand All @@ -41,7 +44,26 @@ public GrpcClientDiagnosticListener(GrpcClientInstrumentationOptions options)
this.options = options;
}

public override void OnStartActivity(Activity activity, object payload)
public override void OnEventWritten(string name, object payload)
{
switch (name)
{
case OnStartEvent:
{
this.OnStartActivity(Activity.Current, payload);
}

break;
case OnStopEvent:
{
this.OnStopActivity(Activity.Current, payload);
}

break;
}
}

public void OnStartActivity(Activity activity, object payload)
{
// The overall flow of what GrpcClient library does is as below:
// Activity.Start()
Expand Down Expand Up @@ -137,7 +159,7 @@ public override void OnStartActivity(Activity activity, object payload)
}
}

public override void OnStopActivity(Activity activity, object payload)
public void OnStopActivity(Activity activity, object payload)
{
if (activity.IsAllDataRequested)
{
Expand Down
Loading

0 comments on commit 8800b2f

Please sign in to comment.