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.

Sign up for GitHub

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

Jump to bottom

MVC use concrete types for DiagnosticListener #11730

Merged
merged 11 commits into from
Jul 15, 2019
Merged

MVC use concrete types for DiagnosticListener #11730

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2360c09
MVC use concrete types for DiagnosticListener
benaadams Jun 30, 2019
5d79e9c
Use DiagnosticListener not DiagnosticSource
benaadams Jul 1, 2019
a85a819
Implement interface explicitly
benaadams Jul 1, 2019
4d4ed86
Code check
benaadams Jul 1, 2019
b617f42
Feedback
benaadams Jul 1, 2019
7da97de
Feedback
benaadams Jul 12, 2019
799b1ed
IReadOnlyDictionary
benaadams Jul 12, 2019
868e217
Remove InstrumentationContext
benaadams Jul 12, 2019
84631cd
DiagnosticSource in RazorPageActivator
benaadams Jul 12, 2019
b569746
Code check
benaadams Jul 12, 2019
e92da57
Feedback
benaadams Jul 15, 2019
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
74 changes: 37 additions & 37 deletions src/Hosting/Hosting/test/HostingApplicationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,13 @@ public void CreateContextWithEnabledLoggerAndRequestIdCreatesActivityAndSetsActi
public void ActivityStopDoesNotFireIfNoListenerAttachedForStart()
{
// Arrange
var diagnosticSource = new DiagnosticListener("DummySource");
var diagnosticListener = new DiagnosticListener("DummySource");
var logger = new LoggerWithScopes(isEnabled: true);
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource, logger: logger);
var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener, logger: logger);
var startFired = false;
var stopFired = false;

diagnosticSource.Subscribe(new CallbackDiagnosticListener(pair =>
diagnosticListener.Subscribe(new CallbackDiagnosticListener(pair =>
{
// This should not fire
if (pair.Key == "Microsoft.AspNetCore.Hosting.HttpRequestIn.Start")
Expand Down Expand Up @@ -128,14 +128,14 @@ public void ActivityStopDoesNotFireIfNoListenerAttachedForStart()
[Fact]
public void ActivityIsNotCreatedWhenIsEnabledForActivityIsFalse()
{
var diagnosticSource = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource);
var diagnosticListener = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);

bool eventsFired = false;
bool isEnabledActivityFired = false;
bool isEnabledStartFired = false;

diagnosticSource.Subscribe(new CallbackDiagnosticListener(pair =>
diagnosticListener.Subscribe(new CallbackDiagnosticListener(pair =>
{
eventsFired |= pair.Key.StartsWith("Microsoft.AspNetCore.Hosting.HttpRequestIn");
}), (s, o, arg3) =>
Expand All @@ -162,14 +162,14 @@ public void ActivityIsNotCreatedWhenIsEnabledForActivityIsFalse()
[Fact]
public void ActivityIsCreatedButNotLoggedWhenIsEnabledForActivityStartIsFalse()
{
var diagnosticSource = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource);
var diagnosticListener = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);

bool eventsFired = false;
bool isEnabledStartFired = false;
bool isEnabledActivityFired = false;

diagnosticSource.Subscribe(new CallbackDiagnosticListener(pair =>
diagnosticListener.Subscribe(new CallbackDiagnosticListener(pair =>
{
eventsFired |= pair.Key.StartsWith("Microsoft.AspNetCore.Hosting.HttpRequestIn");
}), (s, o, arg3) =>
Expand Down Expand Up @@ -199,12 +199,12 @@ public void ActivityIsCreatedButNotLoggedWhenIsEnabledForActivityStartIsFalse()
[Fact]
public void ActivityIsCreatedAndLogged()
{
var diagnosticSource = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource);
var diagnosticListener = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);

bool startCalled = false;

diagnosticSource.Subscribe(new CallbackDiagnosticListener(pair =>
diagnosticListener.Subscribe(new CallbackDiagnosticListener(pair =>
{
if (pair.Key == "Microsoft.AspNetCore.Hosting.HttpRequestIn.Start")
{
Expand All @@ -224,11 +224,11 @@ public void ActivityIsCreatedAndLogged()
[Fact]
public void ActivityIsStoppedDuringStopCall()
{
var diagnosticSource = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource);
var diagnosticListener = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);

bool endCalled = false;
diagnosticSource.Subscribe(new CallbackDiagnosticListener(pair =>
diagnosticListener.Subscribe(new CallbackDiagnosticListener(pair =>
{
if (pair.Key == "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop")
{
Expand All @@ -249,11 +249,11 @@ public void ActivityIsStoppedDuringStopCall()
[Fact]
public void ActivityIsStoppedDuringUnhandledExceptionCall()
{
var diagnosticSource = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource);
var diagnosticListener = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);

bool endCalled = false;
diagnosticSource.Subscribe(new CallbackDiagnosticListener(pair =>
diagnosticListener.Subscribe(new CallbackDiagnosticListener(pair =>
{
if (pair.Key == "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop")
{
Expand All @@ -273,11 +273,11 @@ public void ActivityIsStoppedDuringUnhandledExceptionCall()
[Fact]
public void ActivityIsAvailableDuringUnhandledExceptionCall()
{
var diagnosticSource = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource);
var diagnosticListener = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);

bool endCalled = false;
diagnosticSource.Subscribe(new CallbackDiagnosticListener(pair =>
diagnosticListener.Subscribe(new CallbackDiagnosticListener(pair =>
{
if (pair.Key == "Microsoft.AspNetCore.Hosting.UnhandledException")
{
Expand All @@ -295,10 +295,10 @@ public void ActivityIsAvailableDuringUnhandledExceptionCall()
[Fact]
public void ActivityIsAvailibleDuringRequest()
{
var diagnosticSource = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource);
var diagnosticListener = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);

diagnosticSource.Subscribe(new CallbackDiagnosticListener(pair => { }),
diagnosticListener.Subscribe(new CallbackDiagnosticListener(pair => { }),
s =>
{
if (s.StartsWith("Microsoft.AspNetCore.Hosting.HttpRequestIn"))
Expand All @@ -317,10 +317,10 @@ public void ActivityIsAvailibleDuringRequest()
[Fact]
public void ActivityParentIdAndBaggeReadFromHeaders()
{
var diagnosticSource = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource);
var diagnosticListener = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);

diagnosticSource.Subscribe(new CallbackDiagnosticListener(pair => { }),
diagnosticListener.Subscribe(new CallbackDiagnosticListener(pair => { }),
s =>
{
if (s.StartsWith("Microsoft.AspNetCore.Hosting.HttpRequestIn"))
Expand Down Expand Up @@ -349,10 +349,10 @@ public void ActivityParentIdAndBaggeReadFromHeaders()
[Fact]
public void ActivityTraceParentAndTraceStateFromHeaders()
{
var diagnosticSource = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource);
var diagnosticListener = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);

diagnosticSource.Subscribe(new CallbackDiagnosticListener(pair => { }),
diagnosticListener.Subscribe(new CallbackDiagnosticListener(pair => { }),
s =>
{
if (s.StartsWith("Microsoft.AspNetCore.Hosting.HttpRequestIn"))
Expand Down Expand Up @@ -385,11 +385,11 @@ public void ActivityTraceParentAndTraceStateFromHeaders()
[Fact]
public void ActivityOnExportHookIsCalled()
{
var diagnosticSource = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticSource: diagnosticSource);
var diagnosticListener = new DiagnosticListener("DummySource");
var hostingApplication = CreateApplication(out var features, diagnosticListener: diagnosticListener);

bool onActivityImportCalled = false;
diagnosticSource.Subscribe(
diagnosticListener.Subscribe(
observer: new CallbackDiagnosticListener(pair => { }),
isEnabled: (s, o, _) => true,
onActivityImport: (activity, context) =>
Expand Down Expand Up @@ -422,7 +422,7 @@ private static void AssertProperty<T>(object o, string name)
}

private static HostingApplication CreateApplication(out FeatureCollection features,
DiagnosticListener diagnosticSource = null, ILogger logger = null, Action<DefaultHttpContext> configure = null)
DiagnosticListener diagnosticListener = null, ILogger logger = null, Action<DefaultHttpContext> configure = null)
{
var httpContextFactory = new Mock<IHttpContextFactory>();

Expand All @@ -436,7 +436,7 @@ private static HostingApplication CreateApplication(out FeatureCollection featur
var hostingApplication = new HostingApplication(
ctx => Task.CompletedTask,
logger ?? new NullScopeLogger(),
diagnosticSource ?? new NoopDiagnosticSource(),
diagnosticListener ?? new NoopDiagnosticListener(),
httpContextFactory.Object);

return hostingApplication;
Expand Down Expand Up @@ -490,11 +490,11 @@ public void Dispose()
}
}

private class NoopDiagnosticSource : DiagnosticListener
private class NoopDiagnosticListener : DiagnosticListener
{
private readonly bool _isEnabled;

public NoopDiagnosticSource(bool isEnabled = false) : base("DummyListener")
public NoopDiagnosticListener(bool isEnabled = false) : base("DummyListener")
{
_isEnabled = isEnabled;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Http/src/DefaultHttpContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public override ISession Session

// This property exists because of backwards compatibility.
// We send an anonymous object with an HttpContext property
// via DiagnosticSource in various events throughout the pipeline. Instead
// via DiagnosticListener in various events throughout the pipeline. Instead
rynowak marked this conversation as resolved.
Show resolved Hide resolved
// we just send the HttpContext to avoid extra allocations
[EditorBrowsable(EditorBrowsableState.Never)]
public HttpContext HttpContext => this;
Expand Down
2 changes: 1 addition & 1 deletion src/Middleware/Diagnostics/ref/Microsoft.AspNetCore.Diagnostics.netcoreapp3.0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ namespace Microsoft.AspNetCore.Diagnostics
{
public partial class DeveloperExceptionPageMiddleware
{
public DeveloperExceptionPageMiddleware(Microsoft.AspNetCore.Http.RequestDelegate next, Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Builder.DeveloperExceptionPageOptions> options, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory, Microsoft.AspNetCore.Hosting.IWebHostEnvironment hostingEnvironment, System.Diagnostics.DiagnosticSource diagnosticSource, System.Collections.Generic.IEnumerable<Microsoft.AspNetCore.Diagnostics.IDeveloperPageExceptionFilter> filters) { }
public DeveloperExceptionPageMiddleware(Microsoft.AspNetCore.Http.RequestDelegate next, Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Builder.DeveloperExceptionPageOptions> options, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory, Microsoft.AspNetCore.Hosting.IWebHostEnvironment hostingEnvironment, System.Diagnostics.DiagnosticListener diagnosticListener, System.Collections.Generic.IEnumerable<Microsoft.AspNetCore.Diagnostics.IDeveloperPageExceptionFilter> filters) { }
[System.Diagnostics.DebuggerStepThroughAttribute]
public System.Threading.Tasks.Task Invoke(Microsoft.AspNetCore.Http.HttpContext context) { throw null; }
}
Expand Down
12 changes: 6 additions & 6 deletions src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class DeveloperExceptionPageMiddleware
private readonly DeveloperExceptionPageOptions _options;
private readonly ILogger _logger;
private readonly IFileProvider _fileProvider;
private readonly DiagnosticSource _diagnosticSource;
private readonly DiagnosticListener _diagnosticListerner;
private readonly ExceptionDetailsProvider _exceptionDetailsProvider;
private readonly Func<ErrorContext, Task> _exceptionHandler;
private static readonly MediaTypeHeaderValue _textHtmlMediaType = new MediaTypeHeaderValue("text/html");
Expand All @@ -42,14 +42,14 @@ public class DeveloperExceptionPageMiddleware
/// <param name="options"></param>
/// <param name="loggerFactory"></param>
/// <param name="hostingEnvironment"></param>
/// <param name="diagnosticSource"></param>
/// <param name="diagnosticListener"></param>
/// <param name="filters"></param>
public DeveloperExceptionPageMiddleware(
RequestDelegate next,
IOptions<DeveloperExceptionPageOptions> options,
ILoggerFactory loggerFactory,
IWebHostEnvironment hostingEnvironment,
DiagnosticSource diagnosticSource,
DiagnosticListener diagnosticListener,
benaadams marked this conversation as resolved.
Show resolved Hide resolved
IEnumerable<IDeveloperPageExceptionFilter> filters)
{
if (next == null)
Expand All @@ -71,7 +71,7 @@ public DeveloperExceptionPageMiddleware(
_options = options.Value;
_logger = loggerFactory.CreateLogger<DeveloperExceptionPageMiddleware>();
_fileProvider = _options.FileProvider ?? hostingEnvironment.ContentRootFileProvider;
_diagnosticSource = diagnosticSource;
_diagnosticListerner = diagnosticListener;
_exceptionDetailsProvider = new ExceptionDetailsProvider(_fileProvider, _options.SourceCodeLineCount);
_exceptionHandler = DisplayException;

Expand Down Expand Up @@ -110,9 +110,9 @@ public async Task Invoke(HttpContext context)

await _exceptionHandler(new ErrorContext(context, ex));

if (_diagnosticSource.IsEnabled("Microsoft.AspNetCore.Diagnostics.UnhandledException"))
if (_diagnosticListerner.IsEnabled() && _diagnosticListerner.IsEnabled("Microsoft.AspNetCore.Diagnostics.UnhandledException"))
{
_diagnosticSource.Write("Microsoft.AspNetCore.Diagnostics.UnhandledException", new { httpContext = context, exception = ex });
_diagnosticListerner.Write("Microsoft.AspNetCore.Diagnostics.UnhandledException", new { httpContext = context, exception = ex });
}

return;
Expand Down
2 changes: 1 addition & 1 deletion ...iddleware/MiddlewareAnalysis/ref/Microsoft.AspNetCore.MiddlewareAnalysis.netcoreapp3.0.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public AnalysisBuilder(Microsoft.AspNetCore.Builder.IApplicationBuilder inner) {
}
public partial class AnalysisMiddleware
{
public AnalysisMiddleware(Microsoft.AspNetCore.Http.RequestDelegate next, System.Diagnostics.DiagnosticSource diagnosticSource, string middlewareName) { }
public AnalysisMiddleware(Microsoft.AspNetCore.Http.RequestDelegate next, System.Diagnostics.DiagnosticListener diagnosticListener, string middlewareName) { }
[System.Diagnostics.DebuggerStepThroughAttribute]
public System.Threading.Tasks.Task Invoke(Microsoft.AspNetCore.Http.HttpContext httpContext) { throw null; }
}
Expand Down
12 changes: 6 additions & 6 deletions src/Middleware/MiddlewareAnalysis/src/AnalysisMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ public class AnalysisMiddleware
{
private readonly Guid _instanceId = Guid.NewGuid();
private readonly RequestDelegate _next;
private readonly DiagnosticSource _diagnostics;
private readonly DiagnosticListener _diagnostics;
private readonly string _middlewareName;

public AnalysisMiddleware(RequestDelegate next, DiagnosticSource diagnosticSource, string middlewareName)
public AnalysisMiddleware(RequestDelegate next, DiagnosticListener diagnosticListener, string middlewareName)
{
_next = next;
_diagnostics = diagnosticSource;
_diagnostics = diagnosticListener;
if (string.IsNullOrEmpty(middlewareName))
{
middlewareName = next.Target.GetType().FullName;
Expand All @@ -29,7 +29,7 @@ public AnalysisMiddleware(RequestDelegate next, DiagnosticSource diagnosticSourc
public async Task Invoke(HttpContext httpContext)
{
var startTimestamp = Stopwatch.GetTimestamp();
if (_diagnostics.IsEnabled("Microsoft.AspNetCore.MiddlewareAnalysis.MiddlewareStarting"))
if (_diagnostics.IsEnabled() && _diagnostics.IsEnabled("Microsoft.AspNetCore.MiddlewareAnalysis.MiddlewareStarting"))
{
_diagnostics.Write(
"Microsoft.AspNetCore.MiddlewareAnalysis.MiddlewareStarting",
Expand All @@ -46,7 +46,7 @@ public async Task Invoke(HttpContext httpContext)
{
await _next(httpContext);

if (_diagnostics.IsEnabled("Microsoft.AspNetCore.MiddlewareAnalysis.MiddlewareFinished"))
if (_diagnostics.IsEnabled() && _diagnostics.IsEnabled("Microsoft.AspNetCore.MiddlewareAnalysis.MiddlewareFinished"))
{
var currentTimestamp = Stopwatch.GetTimestamp();
_diagnostics.Write(
Expand All @@ -63,7 +63,7 @@ public async Task Invoke(HttpContext httpContext)
}
catch (Exception ex)
{
if (_diagnostics.IsEnabled("Microsoft.AspNetCore.MiddlewareAnalysis.MiddlewareException"))
if (_diagnostics.IsEnabled() && _diagnostics.IsEnabled("Microsoft.AspNetCore.MiddlewareAnalysis.MiddlewareException"))
{
var currentTimestamp = Stopwatch.GetTimestamp();
_diagnostics.Write(
Expand Down
Loading