diff --git a/CHANGELOG.md b/CHANGELOG.md index 866a2a67..ca68c918 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - [Enables Microsoft.Extensions.Logging.ApplicationInsights.ApplicationInsightsLoggerProvider by default. If ApplicationInsightsLoggerProvider was enabled previously using ILoggerFactory extension method, please remove it to prevent duplicate logs.](https://github.com/Microsoft/ApplicationInsights-aspnetcore/issues/854) - [Remove reference to Microsoft.Extensions.DiagnosticAdapter and use DiagnosticSource subscription APIs directly](https://github.com/Microsoft/ApplicationInsights-aspnetcore/pull/852) - [Fix: NullReferenceException in ApplicationInsightsLogger.Log when exception contains a Data entry with a null value](https://github.com/Microsoft/ApplicationInsights-aspnetcore/issues/848) +- [Performance fixes for GetUri, SetKeyHeaderValue, ConcurrentDictionary use and Telemetry Initializers](https://github.com/Microsoft/ApplicationInsights-aspnetcore/pull/864) ## Version 2.7.0-beta2 - Added NetStandard2.0 target. diff --git a/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HeadersUtilities.cs b/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HeadersUtilities.cs index a10865af..d99c3233 100644 --- a/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HeadersUtilities.cs +++ b/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HeadersUtilities.cs @@ -5,6 +5,7 @@ namespace Microsoft.ApplicationInsights.AspNetCore.DiagnosticListeners using System.Linq; using System.Text.RegularExpressions; using Microsoft.ApplicationInsights.Common; + using Microsoft.Extensions.Primitives; /// /// Generic functions that can be used to get and set Http headers. @@ -20,7 +21,7 @@ internal static class HeadersUtilities public static string GetHeaderKeyValue(IEnumerable headerValues, string keyName) { if (headerValues != null) - { + { foreach (string keyNameValue in headerValues) { string[] keyNameValueParts = keyNameValue.Trim().Split('='); @@ -38,7 +39,7 @@ public static string GetHeaderKeyValue(IEnumerable headerValues, string /// /// Given the provided list of header value strings, return a comma-separated list of key /// name/value pairs with the provided keyName and keyValue. If the initial header value - /// strings contains the key name, then the original key value should be replaced with the + /// string contains the key name, then the original key value should be replaced with the /// provided key value. If the initial header value strings don't contain the key name, /// then the key name/value pair should be added to the comma-separated list and returned. /// @@ -46,14 +47,25 @@ public static string GetHeaderKeyValue(IEnumerable headerValues, string /// The name of the key to add. /// The value of the key to add. /// The result of setting the provided key name/value pair into the provided headerValues. - public static IEnumerable SetHeaderKeyValue(IEnumerable headerValues, string keyName, string keyValue) + public static StringValues SetHeaderKeyValue(string[] currentHeaders, string key, string value) { - string[] newHeaderKeyValue = new[] { string.Format(CultureInfo.InvariantCulture, "{0}={1}", keyName.Trim(), keyValue.Trim()) }; - return headerValues == null || !headerValues.Any() - ? newHeaderKeyValue - : headerValues - .Where(headerValue => !HeaderMatchesKey(headerValue, keyName)) - .Concat(newHeaderKeyValue); + if (currentHeaders != null) + { + for (int index = 0; index < currentHeaders.Length; index++) + { + if (HeaderMatchesKey(currentHeaders[index], key)) + { + currentHeaders[index] = string.Concat(key, "=", value); + return currentHeaders; + } + } + + return StringValues.Concat(currentHeaders, string.Concat(key, "=", value)); + } + else + { + return string.Concat(key, "=", value); + } } /// diff --git a/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HostingDiagnosticListener.cs b/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HostingDiagnosticListener.cs index f90b2d27..d0081195 100644 --- a/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HostingDiagnosticListener.cs +++ b/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HostingDiagnosticListener.cs @@ -161,7 +161,15 @@ public void OnHttpRequestInStart(HttpContext httpContext) // with W3C support on .NET https://github.com/dotnet/corefx/issues/30331 newActivity = new Activity(ActivityCreatedByHostingDiagnosticListener); - newActivity.SetParentId(StringUtilities.GenerateTraceId()); + if (this.enableW3CHeaders) + { + newActivity.GenerateW3CContext(); + newActivity.SetParentId(newActivity.GetTraceId()); + } + else + { + newActivity.SetParentId(W3CUtilities.GenerateTraceId()); + } // end of workaround } @@ -257,7 +265,9 @@ public void OnBeginRequest(HttpContext httpContext, long timestamp) InjectionGuardConstants.RequestHeaderMaxLength); } } - else if(!activity.IsW3CActivity()) + + // no headers + else if (originalParentId == null) { // As a first step in supporting W3C protocol in ApplicationInsights, // we want to generate Activity Ids in the W3C compatible format. @@ -266,8 +276,16 @@ public void OnBeginRequest(HttpContext httpContext, long timestamp) // So if there is no current Activity (i.e. there were no Request-Id header in the incoming request), we'll override ParentId on // the current Activity by the properly formatted one. This workaround should go away // with W3C support on .NET https://github.com/dotnet/corefx/issues/30331 - - activity.SetParentId(StringUtilities.GenerateTraceId()); + + if (this.enableW3CHeaders) + { + activity.GenerateW3CContext(); + activity.SetParentId(activity.GetTraceId()); + } + else + { + activity.SetParentId(W3CUtilities.GenerateTraceId()); + } // end of workaround } @@ -337,6 +355,10 @@ private RequestTelemetry InitializeRequestTelemetry(HttpContext httpContext, Act requestTelemetry.Context.Operation.Id = activity.RootId; requestTelemetry.Id = activity.Id; } + else + { + activity.UpdateTelemetry(requestTelemetry, false); + } foreach (var prop in activity.Baggage) { @@ -346,7 +368,7 @@ private RequestTelemetry InitializeRequestTelemetry(HttpContext httpContext, Act } } - this.client.Initialize(requestTelemetry); + this.client.InitializeInstrumentationKey(requestTelemetry); requestTelemetry.Source = GetAppIdFromRequestHeader(httpContext.Request.Headers, requestTelemetry.Context.InstrumentationKey); @@ -490,10 +512,6 @@ private void SetW3CContext(IHeaderDictionary requestHeaders, Activity activity, InjectionGuardConstants.TraceParentHeaderMaxLength); activity.SetTraceparent(parentTraceParent); } - else - { - activity.GenerateW3CContext(); - } string[] traceStateValues = HttpHeadersUtilities.SafeGetCommaSeparatedHeaderValues(requestHeaders, W3C.W3CConstants.TraceStateHeader, InjectionGuardConstants.TraceStateHeaderMaxLength, InjectionGuardConstants.TraceStateMaxPairs); diff --git a/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HttpHeadersUtilities.cs b/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HttpHeadersUtilities.cs index 4a8bc3ea..d1002896 100644 --- a/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HttpHeadersUtilities.cs +++ b/src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HttpHeadersUtilities.cs @@ -65,28 +65,11 @@ internal static bool ContainsRequestContextKeyValue(IHeaderDictionary headers, s return !string.IsNullOrEmpty(GetHeaderKeyValue(headers, RequestResponseHeaders.RequestContextHeader, keyName)); } - internal static void SetRequestContextKeyValue(HttpHeaders headers, string keyName, string keyValue) - { - SetHeaderKeyValue(headers, RequestResponseHeaders.RequestContextHeader, keyName, keyValue); - } - internal static void SetRequestContextKeyValue(IHeaderDictionary headers, string keyName, string keyValue) { SetHeaderKeyValue(headers, RequestResponseHeaders.RequestContextHeader, keyName, keyValue); } - internal static void SetHeaderKeyValue(HttpHeaders headers, string headerName, string keyName, string keyValue) - { - if (headers == null) - { - throw new ArgumentNullException(nameof(headers)); - } - - IEnumerable headerValues = GetHeaderValues(headers, headerName); - headers.Remove(headerName); - headers.Add(headerName, HeadersUtilities.SetHeaderKeyValue(headerValues, keyName, keyValue)); - } - internal static void SetHeaderKeyValue(IHeaderDictionary headers, string headerName, string keyName, string keyValue) { if (headers == null) @@ -94,7 +77,7 @@ internal static void SetHeaderKeyValue(IHeaderDictionary headers, string headerN throw new ArgumentNullException(nameof(headers)); } - headers[headerName] = new StringValues(HeadersUtilities.SetHeaderKeyValue(headers[headerName].AsEnumerable(), keyName, keyValue).ToArray()); + headers[headerName] = HeadersUtilities.SetHeaderKeyValue(headers[headerName], keyName, keyValue); } internal static string[] SafeGetCommaSeparatedHeaderValues(IHeaderDictionary headers, string headerName, int maxLength, int maxItems) diff --git a/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/HttpRequestExtensions.cs b/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/HttpRequestExtensions.cs index 66479425..1230e15a 100644 --- a/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/HttpRequestExtensions.cs +++ b/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/HttpRequestExtensions.cs @@ -28,26 +28,12 @@ public static Uri GetUri(this HttpRequest request) { throw new ArgumentException("Http request Scheme is not specified"); } - - string hostName = request.Host.HasValue ? request.Host.ToString() : UnknownHostName; - var builder = new StringBuilder(); - - builder.Append(request.Scheme) - .Append("://") - .Append(hostName); - - if (true == request.Path.HasValue) - { - builder.Append(request.Path.Value); - } - - if (true == request.QueryString.HasValue) - { - builder.Append(request.QueryString); - } - - return new Uri(builder.ToString()); + return new Uri(string.Concat(request.Scheme, + "://", + request.Host.HasValue ? request.Host.Value : UnknownHostName, + request.Path.HasValue ? request.Path.Value : "", + request.QueryString.HasValue ? request.QueryString.Value : "")); } } } diff --git a/src/Microsoft.ApplicationInsights.AspNetCore/TelemetryInitializers/AspNetCoreEnvironmentTelemetryInitializer.cs b/src/Microsoft.ApplicationInsights.AspNetCore/TelemetryInitializers/AspNetCoreEnvironmentTelemetryInitializer.cs index c42565d3..1867aec2 100644 --- a/src/Microsoft.ApplicationInsights.AspNetCore/TelemetryInitializers/AspNetCoreEnvironmentTelemetryInitializer.cs +++ b/src/Microsoft.ApplicationInsights.AspNetCore/TelemetryInitializers/AspNetCoreEnvironmentTelemetryInitializer.cs @@ -23,10 +23,10 @@ public AspNetCoreEnvironmentTelemetryInitializer(IHostingEnvironment environment /// public void Initialize(ITelemetry telemetry) - { - if (environment != null && !telemetry.Context.GlobalProperties.ContainsKey(AspNetCoreEnvironmentPropertyName)) + { + if (environment != null && !telemetry.Context.Properties.ContainsKey(AspNetCoreEnvironmentPropertyName)) { - telemetry.Context.GlobalProperties.Add(AspNetCoreEnvironmentPropertyName, environment.EnvironmentName); + telemetry.Context.Properties.Add(AspNetCoreEnvironmentPropertyName, environment.EnvironmentName); } } } diff --git a/test/Microsoft.ApplicationInsights.AspNetCore.Tests/HeadersUtilitiesTest.cs b/test/Microsoft.ApplicationInsights.AspNetCore.Tests/HeadersUtilitiesTest.cs index a771b0aa..fe066f8b 100644 --- a/test/Microsoft.ApplicationInsights.AspNetCore.Tests/HeadersUtilitiesTest.cs +++ b/test/Microsoft.ApplicationInsights.AspNetCore.Tests/HeadersUtilitiesTest.cs @@ -1,7 +1,6 @@ namespace Microsoft.ApplicationInsights.AspNetCore.Tests { using System.Collections.Generic; - using System.Linq; using DiagnosticListeners; using Xunit; @@ -30,36 +29,35 @@ public void ShouldReturnNullWhenKeyNotExists() [Fact] public void ShouldReturnHeadersWhenNoHeaderValues() { - IEnumerable newHeaders = HeadersUtilities.SetHeaderKeyValue(null, "Key", "Value"); + string[] newHeaders = HeadersUtilities.SetHeaderKeyValue(null, "Key", "Value"); Assert.NotNull(newHeaders); - Assert.Equal(1, newHeaders.Count()); - Assert.Equal("Key=Value", newHeaders.First()); + Assert.Single(newHeaders); + Assert.Equal("Key=Value", newHeaders[0]); } [Fact] public void ShouldAppendHeaders() { - IEnumerable existing = new List() { "ExistKey=ExistValue" }; - IEnumerable result = HeadersUtilities.SetHeaderKeyValue(existing, "NewKey", "NewValue"); + string[] existing = new string[] { "ExistKey=ExistValue" }; + string[] result = HeadersUtilities.SetHeaderKeyValue(existing, "NewKey", "NewValue"); Assert.NotNull(result); - Assert.Equal(2, result.Count()); - Assert.NotNull(result.FirstOrDefault(headerValue => headerValue.Equals("ExistKey=ExistValue"))); - Assert.NotNull(result.FirstOrDefault(headerValue => headerValue.Equals("NewKey=NewValue"))); + Assert.Equal(2, result.Length); + Assert.Equal("ExistKey=ExistValue", result[0]); + Assert.Equal("NewKey=NewValue", result[1]); } [Fact] public void ShouldUpdateExistingHeader() { - IEnumerable existing = new List() { "ExistKey=ExistValue", "NoiseKey=NoiseValue" }; - IEnumerable result = HeadersUtilities.SetHeaderKeyValue(existing, "ExistKey", "NewValue"); + string[] existing = new string[] { "ExistKey=ExistValue", "NoiseKey=NoiseValue" }; + string[] result = HeadersUtilities.SetHeaderKeyValue(existing, "ExistKey", "NewValue"); Assert.NotNull(result); - Assert.Equal(2, result.Count()); - Assert.Null(result.FirstOrDefault(headerValue => headerValue.Equals("ExistKey=ExistValue"))); - Assert.NotNull(result.FirstOrDefault(headerValue => headerValue.Equals("ExistKey=NewValue"))); - Assert.NotNull(result.FirstOrDefault(headerValue => headerValue.Equals("NoiseKey=NoiseValue"))); + Assert.Equal(2, result.Length); + Assert.Equal("ExistKey=NewValue", result[0]); + Assert.Equal("NoiseKey=NoiseValue", result[1]); } } } diff --git a/test/Microsoft.ApplicationInsights.AspNetCore.Tests/HostingDiagnosticListenerTest.cs b/test/Microsoft.ApplicationInsights.AspNetCore.Tests/HostingDiagnosticListenerTest.cs index 87ccc82f..bec25163 100644 --- a/test/Microsoft.ApplicationInsights.AspNetCore.Tests/HostingDiagnosticListenerTest.cs +++ b/test/Microsoft.ApplicationInsights.AspNetCore.Tests/HostingDiagnosticListenerTest.cs @@ -836,10 +836,6 @@ public void OnBeginRequestWithW3CSupportAndNoHeadersIsTrackedCorrectly(bool isAs var activityInitializedByW3CHeader = Activity.Current; - if (!isAspNetCore2) - { - Assert.Null(activityInitializedByW3CHeader.ParentId); - } Assert.NotNull(activityInitializedByW3CHeader.GetTraceId()); Assert.Equal(32, activityInitializedByW3CHeader.GetTraceId().Length); Assert.Equal(16, activityInitializedByW3CHeader.GetSpanId().Length); diff --git a/test/Microsoft.ApplicationInsights.AspNetCore.Tests/TelemetryInitializers/AspNetCoreEnvironmentTelemetryInitializerTests.cs b/test/Microsoft.ApplicationInsights.AspNetCore.Tests/TelemetryInitializers/AspNetCoreEnvironmentTelemetryInitializerTests.cs index 783199dc..7fa681fd 100644 --- a/test/Microsoft.ApplicationInsights.AspNetCore.Tests/TelemetryInitializers/AspNetCoreEnvironmentTelemetryInitializerTests.cs +++ b/test/Microsoft.ApplicationInsights.AspNetCore.Tests/TelemetryInitializers/AspNetCoreEnvironmentTelemetryInitializerTests.cs @@ -19,10 +19,10 @@ public void InitializeDoesNotOverrideExistingProperty() { var initializer = new AspNetCoreEnvironmentTelemetryInitializer(new HostingEnvironment() { EnvironmentName = "Production"}); var telemetry = new RequestTelemetry(); - telemetry.Context.GlobalProperties.Add("AspNetCoreEnvironment", "Development"); + telemetry.Context.Properties.Add("AspNetCoreEnvironment", "Development"); initializer.Initialize(telemetry); - Assert.Equal("Development", telemetry.Context.GlobalProperties["AspNetCoreEnvironment"]); + Assert.Equal("Development", telemetry.Context.Properties["AspNetCoreEnvironment"]); } [Fact] @@ -32,7 +32,7 @@ public void InitializeSetsCurrentEnvironmentNameToProperty() var telemetry = new RequestTelemetry(); initializer.Initialize(telemetry); - Assert.Equal("Production", telemetry.Context.GlobalProperties["AspNetCoreEnvironment"]); + Assert.Equal("Production", telemetry.Context.Properties["AspNetCoreEnvironment"]); } } }