From fea15eb8de4bc36debe32e788e8ab5868ea374df Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Fri, 10 Jul 2015 09:53:58 -0700 Subject: [PATCH] Removed creation of request id logging scope --- .gitignore | 1 + DiagnosticsPages.sln | 17 ++- samples/ElmPageSample/ElmPageSample.xproj | 19 +++ samples/ElmPageSample/HelloWorldMiddleware.cs | 34 +++++ samples/ElmPageSample/Startup.cs | 31 +++++ samples/ElmPageSample/project.json | 16 +++ samples/ElmPageSample/wwwroot/Readme.md | 1 + .../ElmCaptureMiddleware.cs | 28 +++-- .../HttpInfo.cs | 4 +- .../HttpRequestIdentifierFeature.cs | 12 ++ .../RequestIdentifier.cs | 57 +++++++++ .../Views/DetailsPage.cs | 1 + .../Views/LogPage.cs | 1 + .../ElmMiddlewareTest.cs | 117 +++++++++++++++++- 14 files changed, 322 insertions(+), 17 deletions(-) create mode 100644 samples/ElmPageSample/ElmPageSample.xproj create mode 100644 samples/ElmPageSample/HelloWorldMiddleware.cs create mode 100644 samples/ElmPageSample/Startup.cs create mode 100644 samples/ElmPageSample/project.json create mode 100644 samples/ElmPageSample/wwwroot/Readme.md create mode 100644 src/Microsoft.AspNet.Diagnostics.Elm/HttpRequestIdentifierFeature.cs create mode 100644 src/Microsoft.AspNet.Diagnostics.Elm/RequestIdentifier.cs diff --git a/.gitignore b/.gitignore index 7b3d14ea93..801273e9db 100644 --- a/.gitignore +++ b/.gitignore @@ -27,3 +27,4 @@ nuget.exe node_modules *.sln.ide project.lock.json +.vs/ \ No newline at end of file diff --git a/DiagnosticsPages.sln b/DiagnosticsPages.sln index f893c1dfb7..05042e2643 100644 --- a/DiagnosticsPages.sln +++ b/DiagnosticsPages.sln @@ -1,7 +1,7 @@  Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 14 -VisualStudioVersion = 14.0.22823.1 +VisualStudioVersion = 14.0.23107.0 MinimumVisualStudioVersion = 10.0.40219.1 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{509A6F36-AD80-4A18-B5B1-717D38DFF29D}" EndProject @@ -40,6 +40,8 @@ Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "StatusCodePagesSample", "sa EndProject Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNet.Diagnostics.Abstractions", "src\Microsoft.AspNet.Diagnostics.Abstractions\Microsoft.AspNet.Diagnostics.Abstractions.xproj", "{83FFB65A-97B1-45AA-BCB8-3F43966BC8A3}" EndProject +Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "ElmPageSample", "samples\ElmPageSample\ElmPageSample.xproj", "{FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -192,6 +194,18 @@ Global {83FFB65A-97B1-45AA-BCB8-3F43966BC8A3}.Release|Mixed Platforms.Build.0 = Release|Any CPU {83FFB65A-97B1-45AA-BCB8-3F43966BC8A3}.Release|x86.ActiveCfg = Release|Any CPU {83FFB65A-97B1-45AA-BCB8-3F43966BC8A3}.Release|x86.Build.0 = Release|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Debug|Any CPU.Build.0 = Debug|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Debug|x86.ActiveCfg = Debug|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Debug|x86.Build.0 = Debug|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Release|Any CPU.ActiveCfg = Release|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Release|Any CPU.Build.0 = Release|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Release|x86.ActiveCfg = Release|Any CPU + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -210,5 +224,6 @@ Global {624B0019-956A-4157-B008-270C5B229553} = {509A6F36-AD80-4A18-B5B1-717D38DFF29D} {CC1F5841-FE10-4DDB-8477-C4DE92BA759F} = {ACAA0157-A8C4-4152-93DE-90CCDF304087} {83FFB65A-97B1-45AA-BCB8-3F43966BC8A3} = {509A6F36-AD80-4A18-B5B1-717D38DFF29D} + {FFD28DCF-C24F-4C59-9B6B-F3B74CE13129} = {ACAA0157-A8C4-4152-93DE-90CCDF304087} EndGlobalSection EndGlobal diff --git a/samples/ElmPageSample/ElmPageSample.xproj b/samples/ElmPageSample/ElmPageSample.xproj new file mode 100644 index 0000000000..2edc086950 --- /dev/null +++ b/samples/ElmPageSample/ElmPageSample.xproj @@ -0,0 +1,19 @@ + + + + 14.0 + $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion) + + + + ffd28dcf-c24f-4c59-9b6b-f3b74ce13129 + ElmPageSample + ..\..\artifacts\obj\$(MSBuildProjectName) + ..\..\artifacts\bin\$(MSBuildProjectName)\ + + + 2.0 + 42364 + + + \ No newline at end of file diff --git a/samples/ElmPageSample/HelloWorldMiddleware.cs b/samples/ElmPageSample/HelloWorldMiddleware.cs new file mode 100644 index 0000000000..f870a9d210 --- /dev/null +++ b/samples/ElmPageSample/HelloWorldMiddleware.cs @@ -0,0 +1,34 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Microsoft.AspNet.Builder; +using Microsoft.AspNet.Http; +using Microsoft.Framework.Logging; + +namespace ElmPageSample +{ + public class HelloWorldMiddleware + { + private readonly ILogger _logger; + private readonly RequestDelegate _next; + + public HelloWorldMiddleware(RequestDelegate next, ILoggerFactory loggerFactory) + { + _next = next; + _logger = loggerFactory.CreateLogger(); + } + + public async Task Invoke(HttpContext httpContext) + { + using (_logger.BeginScope("Scope1")) + { + _logger.LogVerbose("Getting message"); + + httpContext.Response.ContentType = "text/html; charset=utf-8"; + await httpContext.Response.WriteAsync( + "

Hello World!

Elm Logs"); + } + } + } +} \ No newline at end of file diff --git a/samples/ElmPageSample/Startup.cs b/samples/ElmPageSample/Startup.cs new file mode 100644 index 0000000000..a282c11fbe --- /dev/null +++ b/samples/ElmPageSample/Startup.cs @@ -0,0 +1,31 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNet.Builder; +using Microsoft.Framework.DependencyInjection; +using Microsoft.Framework.Logging; + +namespace ElmPageSample +{ + public class Startup + { + public void ConfigureServices(IServiceCollection services) + { + services.AddElm(); + + services.ConfigureElm(elmOptions => + { + elmOptions.Filter = (loggerName, loglevel) => loglevel == LogLevel.Verbose; + }); + } + + public void Configure(IApplicationBuilder app) + { + app.UseElmPage(); + + app.UseElmCapture(); + + app.UseMiddleware(); + } + } +} diff --git a/samples/ElmPageSample/project.json b/samples/ElmPageSample/project.json new file mode 100644 index 0000000000..f32a35eaa4 --- /dev/null +++ b/samples/ElmPageSample/project.json @@ -0,0 +1,16 @@ +{ + "webroot": "wwwroot", + "exclude": "wwwroot/**/*.*", + "dependencies": { + "Microsoft.AspNet.Diagnostics.Elm": "1.0.0-*", + "Microsoft.AspNet.Server.IIS": "1.0.0-*", + "Microsoft.AspNet.Server.WebListener": "1.0.0-*" + }, + "commands": { + "web": "Microsoft.AspNet.Hosting --server Microsoft.AspNet.Server.WebListener --server.urls http://localhost:5000" + }, + "frameworks": { + "dnx451" : { }, + "dnxcore50" : { } + } +} diff --git a/samples/ElmPageSample/wwwroot/Readme.md b/samples/ElmPageSample/wwwroot/Readme.md new file mode 100644 index 0000000000..11abaf5374 --- /dev/null +++ b/samples/ElmPageSample/wwwroot/Readme.md @@ -0,0 +1 @@ +Sample demonstrating ElmPageMiddleware \ No newline at end of file diff --git a/src/Microsoft.AspNet.Diagnostics.Elm/ElmCaptureMiddleware.cs b/src/Microsoft.AspNet.Diagnostics.Elm/ElmCaptureMiddleware.cs index 72186fa6a1..4bfcf351da 100644 --- a/src/Microsoft.AspNet.Diagnostics.Elm/ElmCaptureMiddleware.cs +++ b/src/Microsoft.AspNet.Diagnostics.Elm/ElmCaptureMiddleware.cs @@ -1,10 +1,10 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Threading.Tasks; using Microsoft.AspNet.Builder; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Features; using Microsoft.Framework.Logging; using Microsoft.Framework.OptionsModel; @@ -28,18 +28,20 @@ namespace Microsoft.AspNet.Diagnostics.Elm public async Task Invoke(HttpContext context) { - var requestId = Guid.NewGuid(); - using (_logger.BeginScope(string.Format("request {0}", requestId))) + using (RequestIdentifier.Ensure(context)) { - var p = ElmScope.Current; - ElmScope.Current.Context.HttpInfo = GetHttpInfo(context, requestId); - try + var requestId = context.GetFeature().TraceIdentifier; + using (_logger.BeginScope("Request: {RequestId}", requestId)) { - await _next(context); - } - finally - { - ElmScope.Current.Context.HttpInfo.StatusCode = context.Response.StatusCode; + try + { + ElmScope.Current.Context.HttpInfo = GetHttpInfo(context); + await _next(context); + } + finally + { + ElmScope.Current.Context.HttpInfo.StatusCode = context.Response.StatusCode; + } } } } @@ -48,11 +50,11 @@ namespace Microsoft.AspNet.Diagnostics.Elm /// Takes the info from the given HttpContext and copies it to an HttpInfo object /// /// The HttpInfo for the current elm context - private static HttpInfo GetHttpInfo(HttpContext context, Guid requestId) + private static HttpInfo GetHttpInfo(HttpContext context) { return new HttpInfo() { - RequestID = requestId, + RequestID = context.GetFeature().TraceIdentifier, Host = context.Request.Host, ContentType = context.Request.ContentType, Path = context.Request.Path, diff --git a/src/Microsoft.AspNet.Diagnostics.Elm/HttpInfo.cs b/src/Microsoft.AspNet.Diagnostics.Elm/HttpInfo.cs index fc6becdb99..dee00626af 100644 --- a/src/Microsoft.AspNet.Diagnostics.Elm/HttpInfo.cs +++ b/src/Microsoft.AspNet.Diagnostics.Elm/HttpInfo.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNet.Diagnostics.Elm { public class HttpInfo { - public Guid RequestID { get; set; } + public string RequestID { get; set; } public HostString Host { get; set; } @@ -20,7 +20,7 @@ namespace Microsoft.AspNet.Diagnostics.Elm public string Scheme { get; set; } public int StatusCode { get; set; } - + public ClaimsPrincipal User { get; set; } public string Method { get; set; } diff --git a/src/Microsoft.AspNet.Diagnostics.Elm/HttpRequestIdentifierFeature.cs b/src/Microsoft.AspNet.Diagnostics.Elm/HttpRequestIdentifierFeature.cs new file mode 100644 index 0000000000..213c711d6b --- /dev/null +++ b/src/Microsoft.AspNet.Diagnostics.Elm/HttpRequestIdentifierFeature.cs @@ -0,0 +1,12 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNet.Http.Features; + +namespace Microsoft.AspNet.Diagnostics.Elm +{ + internal class HttpRequestIdentifierFeature : IHttpRequestIdentifierFeature + { + public string TraceIdentifier { get; set; } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Diagnostics.Elm/RequestIdentifier.cs b/src/Microsoft.AspNet.Diagnostics.Elm/RequestIdentifier.cs new file mode 100644 index 0000000000..0ec1d2e2b2 --- /dev/null +++ b/src/Microsoft.AspNet.Diagnostics.Elm/RequestIdentifier.cs @@ -0,0 +1,57 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Features; + +namespace Microsoft.AspNet.Diagnostics.Elm +{ + internal class RequestIdentifier : IDisposable + { + private readonly bool _addedFeature; + private readonly bool _updatedIdentifier; + private readonly string _originalIdentifierValue; + private readonly HttpContext _context; + private readonly IHttpRequestIdentifierFeature _feature; + + private RequestIdentifier(HttpContext context) + { + _context = context; + _feature = context.GetFeature(); + + if (_feature == null) + { + _feature = new HttpRequestIdentifierFeature() + { + TraceIdentifier = Guid.NewGuid().ToString() + }; + context.SetFeature(_feature); + _addedFeature = true; + } + else if (string.IsNullOrEmpty(_feature.TraceIdentifier)) + { + _originalIdentifierValue = _feature.TraceIdentifier; + _feature.TraceIdentifier = Guid.NewGuid().ToString(); + _updatedIdentifier = true; + } + } + + public static IDisposable Ensure(HttpContext context) + { + return new RequestIdentifier(context); + } + + public void Dispose() + { + if (_addedFeature) + { + _context.SetFeature(null); + } + else if (_updatedIdentifier) + { + _feature.TraceIdentifier = _originalIdentifierValue; + } + } + } +} diff --git a/src/Microsoft.AspNet.Diagnostics.Elm/Views/DetailsPage.cs b/src/Microsoft.AspNet.Diagnostics.Elm/Views/DetailsPage.cs index 52d3684656..5b7705542e 100644 --- a/src/Microsoft.AspNet.Diagnostics.Elm/Views/DetailsPage.cs +++ b/src/Microsoft.AspNet.Diagnostics.Elm/Views/DetailsPage.cs @@ -240,6 +240,7 @@ WriteTo(__razor_helper_writer, Traverse(node.Children[i])); #pragma warning disable 1998 public override async Task ExecuteAsync() { + Response.ContentType = "text/html; charset=utf-8"; WriteLiteral("\r\n"); WriteLiteral("\r\n"); WriteLiteral("\r\n"); diff --git a/src/Microsoft.AspNet.Diagnostics.Elm/Views/LogPage.cs b/src/Microsoft.AspNet.Diagnostics.Elm/Views/LogPage.cs index be1ab6f12f..81a30ca1ec 100644 --- a/src/Microsoft.AspNet.Diagnostics.Elm/Views/LogPage.cs +++ b/src/Microsoft.AspNet.Diagnostics.Elm/Views/LogPage.cs @@ -306,6 +306,7 @@ WriteTo(__razor_helper_writer, LogRow(new LogInfo() #pragma warning disable 1998 public override async Task ExecuteAsync() { + Response.ContentType = "text/html; charset=utf-8"; WriteLiteral("\r\n"); WriteLiteral("\r\n\r\n"); WriteLiteral("\r\n"); diff --git a/test/Microsoft.AspNet.Diagnostics.Tests/ElmMiddlewareTest.cs b/test/Microsoft.AspNet.Diagnostics.Tests/ElmMiddlewareTest.cs index d76d696a1a..f2f248f6dc 100644 --- a/test/Microsoft.AspNet.Diagnostics.Tests/ElmMiddlewareTest.cs +++ b/test/Microsoft.AspNet.Diagnostics.Tests/ElmMiddlewareTest.cs @@ -1,13 +1,18 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.IO; using System.Security.Claims; using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Builder; using Microsoft.AspNet.Diagnostics.Elm; +using Microsoft.AspNet.FeatureModel; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Features; +using Microsoft.AspNet.Http.Features.Internal; +using Microsoft.AspNet.Http.Internal; using Microsoft.Framework.Logging; using Microsoft.Framework.OptionsModel; #if DNX451 @@ -213,9 +218,119 @@ namespace Microsoft.AspNet.Diagnostics.Tests contextMock .Setup(c => c.Request.HasFormContentType) .Returns(true); - + var requestIdentifier = new Mock(); + requestIdentifier.Setup(f => f.TraceIdentifier).Returns(Guid.NewGuid().ToString()); + contextMock.Setup(c => c.GetFeature()) + .Returns(requestIdentifier.Object); return contextMock; } #endif + + [Fact] + public async Task SetsNewIdentifierFeature_IfNotPresentOnContext() + { + // Arrange + var context = new DefaultHttpContext(); + var loggerFactory = new LoggerFactory(); + loggerFactory.AddProvider(new ElmLoggerProvider(new ElmStore(), new ElmOptions())); + + // Act & Assert + var errorPageMiddleware = new ElmCaptureMiddleware((innerContext) => + { + var feature = innerContext.GetFeature(); + Assert.NotNull(feature); + Assert.False(string.IsNullOrEmpty(feature.TraceIdentifier)); + return Task.FromResult(0); + }, loggerFactory, new TestElmOptions()); + + await errorPageMiddleware.Invoke(context); + + Assert.Null(context.GetFeature()); + } + + [Fact] + public async Task UsesIdentifierFeature_IfAlreadyPresentOnContext() + { + // Arrange + var requestIdentifierFeature = new HttpRequestIdentifierFeature() + { + TraceIdentifier = Guid.NewGuid().ToString() + }; + var features = new FeatureCollection(); + features.Add(typeof(IHttpRequestFeature), new HttpRequestFeature()); + features.Add(typeof(IHttpRequestIdentifierFeature), requestIdentifierFeature); + features.Add(typeof(IHttpResponseFeature), new HttpResponseFeature()); + var context = new DefaultHttpContext(features); + var loggerFactory = new LoggerFactory(); + loggerFactory.AddProvider(new ElmLoggerProvider(new ElmStore(), new ElmOptions())); + + // Act & Assert + var errorPageMiddleware = new ElmCaptureMiddleware((innerContext) => + { + Assert.Same(requestIdentifierFeature, innerContext.GetFeature()); + return Task.FromResult(0); + }, loggerFactory, new TestElmOptions()); + + await errorPageMiddleware.Invoke(context); + + Assert.Same(requestIdentifierFeature, context.GetFeature()); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + public async Task UpdatesTraceIdentifier_IfNullOrEmpty(string requestId) + { + // Arrange + var requestIdentifierFeature = new HttpRequestIdentifierFeature() { TraceIdentifier = requestId }; + var features = new FeatureCollection(); + features.Add(typeof(IHttpRequestIdentifierFeature), requestIdentifierFeature); + features.Add(typeof(IHttpRequestFeature), new HttpRequestFeature()); + features.Add(typeof(IHttpResponseFeature), new HttpResponseFeature()); + var context = new DefaultHttpContext(features); + var loggerFactory = new LoggerFactory(); + loggerFactory.AddProvider(new ElmLoggerProvider(new ElmStore(), new ElmOptions())); + + // Act & Assert + var errorPageMiddleware = new ElmCaptureMiddleware((innerContext) => + { + var feature = innerContext.GetFeature(); + Assert.NotNull(feature); + Assert.False(string.IsNullOrEmpty(feature.TraceIdentifier)); + return Task.FromResult(0); + }, loggerFactory, new TestElmOptions()); + + await errorPageMiddleware.Invoke(context); + + Assert.Equal(requestId, context.GetFeature().TraceIdentifier); + } + + private class TestElmOptions : IOptions + { + private readonly ElmOptions _innerOptions; + + public TestElmOptions() : + this(new ElmOptions()) + { + } + + public TestElmOptions(ElmOptions innerOptions) + { + _innerOptions = innerOptions; + } + + public ElmOptions Options + { + get + { + return _innerOptions; + } + } + + public ElmOptions GetNamedOptions(string name) + { + return _innerOptions; + } + } } }