Don't fire DiagnosticSource.StopActivity in some cases (#9595)

- Don't fire the event if DiagnosticSource.StartActivity was never called. This avoids allocating the anonymous object and extra strings.
This commit is contained in:
David Fowler 2019-04-21 16:09:40 -07:00 committed by GitHub
parent f0ea4fc8a1
commit 667f7ca491
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 61 additions and 5 deletions

View File

@ -62,6 +62,7 @@ namespace Microsoft.AspNetCore.Hosting.Internal
public long StartTimestamp { get; set; }
public bool EventLogEnabled { get; set; }
public Activity Activity { get; set; }
internal bool HasDiagnosticListener { get; set; }
}
}
}

View File

@ -48,7 +48,8 @@ namespace Microsoft.AspNetCore.Hosting.Internal
if (loggingEnabled || (diagnosticListenerEnabled && _diagnosticListener.IsEnabled(ActivityName, httpContext)))
{
context.Activity = StartActivity(httpContext);
context.Activity = StartActivity(httpContext, out var hasDiagnosticListener);
context.HasDiagnosticListener = hasDiagnosticListener;
}
if (diagnosticListenerEnabled)
@ -132,7 +133,7 @@ namespace Microsoft.AspNetCore.Hosting.Internal
// Always stop activity if it was started
if (activity != null)
{
StopActivity(httpContext, activity);
StopActivity(httpContext, activity, context.HasDiagnosticListener);
}
if (context.EventLogEnabled && exception != null)
@ -229,9 +230,10 @@ namespace Microsoft.AspNetCore.Hosting.Internal
}
[MethodImpl(MethodImplOptions.NoInlining)]
private Activity StartActivity(HttpContext httpContext)
private Activity StartActivity(HttpContext httpContext, out bool hasDiagnosticListener)
{
var activity = new Activity(ActivityName);
hasDiagnosticListener = false;
var headers = httpContext.Request.Headers;
if (!headers.TryGetValue(HeaderNames.TraceParent, out var requestId))
@ -266,6 +268,7 @@ namespace Microsoft.AspNetCore.Hosting.Internal
if (_diagnosticListener.IsEnabled(ActivityStartKey))
{
hasDiagnosticListener = true;
_diagnosticListener.StartActivity(activity, new { HttpContext = httpContext });
}
else
@ -277,9 +280,16 @@ namespace Microsoft.AspNetCore.Hosting.Internal
}
[MethodImpl(MethodImplOptions.NoInlining)]
private void StopActivity(HttpContext httpContext, Activity activity)
private void StopActivity(HttpContext httpContext, Activity activity, bool hasDiagnosticListener)
{
_diagnosticListener.StopActivity(activity, new { HttpContext = httpContext });
if (hasDiagnosticListener)
{
_diagnosticListener.StopActivity(activity, new { HttpContext = httpContext });
}
else
{
activity.Stop();
}
}
}
}

View File

@ -56,6 +56,51 @@ namespace Microsoft.AspNetCore.Hosting.Tests
Assert.Equal(Activity.Current.Id, pairs["ActivityId"].ToString());
}
[Fact]
public void ActivityStopDoesNotFireIfNoListenerAttachedForStart()
{
// Arrange
var diagnosticSource = new DiagnosticListener("DummySource");
var logger = new LoggerWithScopes(isEnabled: true);
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource, logger: logger);
var startFired = false;
var stopFired = false;
diagnosticSource.Subscribe(new CallbackDiagnosticListener(pair =>
{
// This should not fire
if (pair.Key == "Microsoft.AspNetCore.Hosting.HttpRequestIn.Start")
{
startFired = true;
}
// This should not fire
if (pair.Key == "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop")
{
stopFired = true;
}
}),
(s, o, arg3) =>
{
// The events are off
return false;
});
// Act
var context = hostingApplication.CreateContext(features);
Assert.Single(logger.Scopes);
var pairs = ((IReadOnlyList<KeyValuePair<string, object>>)logger.Scopes[0]).ToDictionary(p => p.Key, p => p.Value);
Assert.Equal(Activity.Current.Id, pairs["ActivityId"].ToString());
hostingApplication.DisposeContext(context, exception: null);
Assert.False(startFired);
Assert.False(stopFired);
Assert.Null(Activity.Current);
}
[Fact]
public void ActivityIsNotCreatedWhenIsEnabledForActivityIsFalse()
{