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

Modify ASP.NET Core instrumentation to take advantage of sampling decision #1899

4 changes: 4 additions & 0 deletions src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
and ActivityProcessors. Samplers, ActivityProcessor.OnStart will now get the
Activity before any enrichment done by the instrumentation.
([#1836](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1836))
* Performance optimization by leveraging sampling decision and short circuiting
activity enrichment. `Filter` and `Enrich` are now only called if
`activity.IsAllDataRequested` is `true`
([#1899](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1899))

## 1.0.0-rc2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,29 +57,29 @@ public HttpInListener(string name, AspNetCoreInstrumentationOptions options)
[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)
{
_ = this.startContextFetcher.TryFetch(payload, out HttpContext context);
if (context == null)
// The overall flow of what AspNetCore library does is as below:
// Activity.Start()
// DiagnosticSource.WriteEvent("Start", payload)
// DiagnosticSource.WriteEvent("Stop", payload)
// Activity.Stop()

// This method is in the WriteEvent("Start", payload) path.
// By this time, samplers have already run and
// activity.IsAllDataRequested populated accordingly.

if (Sdk.SuppressInstrumentation)
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
{
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStartActivity));
return;
}

try
{
if (this.options.Filter?.Invoke(context) == false)
{
AspNetCoreInstrumentationEventSource.Log.RequestIsFilteredOut(activity.OperationName);
activity.IsAllDataRequested = false;
return;
}
}
catch (Exception ex)
_ = this.startContextFetcher.TryFetch(payload, out HttpContext context);
if (context == null)
{
AspNetCoreInstrumentationEventSource.Log.RequestFilterException(ex);
activity.IsAllDataRequested = false;
AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStartActivity));
return;
}

// Ensure context extraction irrespective of sampling decision
var request = context.Request;
var textMapPropagator = Propagators.DefaultTextMapPropagator;
if (!this.hostingSupportsW3C || !(textMapPropagator is TraceContextPropagator))
Expand Down Expand Up @@ -110,11 +110,29 @@ public override void OnStartActivity(Activity activity, object payload)
}
}

ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource);
ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Server);

// enrich Activity from payload only if sampling decision
// is favorable.
if (activity.IsAllDataRequested)
{
try
{
if (this.options.Filter?.Invoke(context) == false)
utpilla marked this conversation as resolved.
Show resolved Hide resolved
{
AspNetCoreInstrumentationEventSource.Log.RequestIsFilteredOut(activity.OperationName);
activity.IsAllDataRequested = false;
return;
}
}
catch (Exception ex)
{
AspNetCoreInstrumentationEventSource.Log.RequestFilterException(ex);
activity.IsAllDataRequested = false;
return;
}

ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource);
ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Server);

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

Expand Down
Loading