Skip to content
This repository was archived by the owner on Jun 10, 2020. It is now read-only.

Commit 4e05f2a

Browse files
Dmitry-MatveevDmitry-Matveev
authored
Performance fixes, wave 1. (#864)
* Remove DiagnosticAdapter and use DiagnosticListener directly * minor fixes * switch * Initialize IKey * Fix typo * GetUri with Concat * Set Header fixes, use Properties for AspNetCoreEnv. * SetHeader fixes * Test Fixes - 1 * fix tests * Keep Public vs. Internal * Changelog update
1 parent 883589c commit 4e05f2a

File tree

9 files changed

+74
-80
lines changed

9 files changed

+74
-80
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
- [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)
55
- [Remove reference to Microsoft.Extensions.DiagnosticAdapter and use DiagnosticSource subscription APIs directly](https://github.com/Microsoft/ApplicationInsights-aspnetcore/pull/852)
66
- [Fix: NullReferenceException in ApplicationInsightsLogger.Log when exception contains a Data entry with a null value](https://github.com/Microsoft/ApplicationInsights-aspnetcore/issues/848)
7+
- [Performance fixes for GetUri, SetKeyHeaderValue, ConcurrentDictionary use and Telemetry Initializers](https://github.com/Microsoft/ApplicationInsights-aspnetcore/pull/864)
78

89
## Version 2.7.0-beta2
910
- Added NetStandard2.0 target.

‎src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HeadersUtilities.cs‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ namespace Microsoft.ApplicationInsights.AspNetCore.DiagnosticListeners
55
using System.Linq;
66
using System.Text.RegularExpressions;
77
using Microsoft.ApplicationInsights.Common;
8+
using Microsoft.Extensions.Primitives;
89

910
/// <summary>
1011
/// Generic functions that can be used to get and set Http headers.
@@ -20,7 +21,7 @@ internal static class HeadersUtilities
2021
public static string GetHeaderKeyValue(IEnumerable<string> headerValues, string keyName)
2122
{
2223
if (headerValues != null)
23-
{
24+
{
2425
foreach (string keyNameValue in headerValues)
2526
{
2627
string[] keyNameValueParts = keyNameValue.Trim().Split('=');
@@ -38,22 +39,33 @@ public static string GetHeaderKeyValue(IEnumerable<string> headerValues, string
3839
/// <summary>
3940
/// Given the provided list of header value strings, return a comma-separated list of key
4041
/// name/value pairs with the provided keyName and keyValue. If the initial header value
41-
/// strings contains the key name, then the original key value should be replaced with the
42+
/// string contains the key name, then the original key value should be replaced with the
4243
/// provided key value. If the initial header value strings don't contain the key name,
4344
/// then the key name/value pair should be added to the comma-separated list and returned.
4445
/// </summary>
4546
/// <param name="headerValues">The existing header values that the key/value pair should be added to.</param>
4647
/// <param name="keyName">The name of the key to add.</param>
4748
/// <param name="keyValue">The value of the key to add.</param>
4849
/// <returns>The result of setting the provided key name/value pair into the provided headerValues.</returns>
49-
public static IEnumerable<string> SetHeaderKeyValue(IEnumerable<string> headerValues, string keyName, string keyValue)
50+
public static StringValues SetHeaderKeyValue(string[] currentHeaders, string key, string value)
5051
{
51-
string[] newHeaderKeyValue = new[] { string.Format(CultureInfo.InvariantCulture, "{0}={1}", keyName.Trim(), keyValue.Trim()) };
52-
return headerValues == null || !headerValues.Any()
53-
? newHeaderKeyValue
54-
: headerValues
55-
.Where(headerValue => !HeaderMatchesKey(headerValue, keyName))
56-
.Concat(newHeaderKeyValue);
52+
if (currentHeaders != null)
53+
{
54+
for (int index = 0; index < currentHeaders.Length; index++)
55+
{
56+
if (HeaderMatchesKey(currentHeaders[index], key))
57+
{
58+
currentHeaders[index] = string.Concat(key, "=", value);
59+
return currentHeaders;
60+
}
61+
}
62+
63+
return StringValues.Concat(currentHeaders, string.Concat(key, "=", value));
64+
}
65+
else
66+
{
67+
return string.Concat(key, "=", value);
68+
}
5769
}
5870

5971
/// <summary>

‎src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HostingDiagnosticListener.cs‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,15 @@ public void OnHttpRequestInStart(HttpContext httpContext)
161161
// with W3C support on .NET https://github.com/dotnet/corefx/issues/30331
162162

163163
newActivity = new Activity(ActivityCreatedByHostingDiagnosticListener);
164-
newActivity.SetParentId(StringUtilities.GenerateTraceId());
164+
if (this.enableW3CHeaders)
165+
{
166+
newActivity.GenerateW3CContext();
167+
newActivity.SetParentId(newActivity.GetTraceId());
168+
}
169+
else
170+
{
171+
newActivity.SetParentId(W3CUtilities.GenerateTraceId());
172+
}
165173
// end of workaround
166174
}
167175

@@ -257,7 +265,9 @@ public void OnBeginRequest(HttpContext httpContext, long timestamp)
257265
InjectionGuardConstants.RequestHeaderMaxLength);
258266
}
259267
}
260-
else if(!activity.IsW3CActivity())
268+
269+
// no headers
270+
else if (originalParentId == null)
261271
{
262272
// As a first step in supporting W3C protocol in ApplicationInsights,
263273
// we want to generate Activity Ids in the W3C compatible format.
@@ -266,8 +276,16 @@ public void OnBeginRequest(HttpContext httpContext, long timestamp)
266276
// So if there is no current Activity (i.e. there were no Request-Id header in the incoming request), we'll override ParentId on
267277
// the current Activity by the properly formatted one. This workaround should go away
268278
// with W3C support on .NET https://github.com/dotnet/corefx/issues/30331
269-
270-
activity.SetParentId(StringUtilities.GenerateTraceId());
279+
280+
if (this.enableW3CHeaders)
281+
{
282+
activity.GenerateW3CContext();
283+
activity.SetParentId(activity.GetTraceId());
284+
}
285+
else
286+
{
287+
activity.SetParentId(W3CUtilities.GenerateTraceId());
288+
}
271289

272290
// end of workaround
273291
}
@@ -337,6 +355,10 @@ private RequestTelemetry InitializeRequestTelemetry(HttpContext httpContext, Act
337355
requestTelemetry.Context.Operation.Id = activity.RootId;
338356
requestTelemetry.Id = activity.Id;
339357
}
358+
else
359+
{
360+
activity.UpdateTelemetry(requestTelemetry, false);
361+
}
340362

341363
foreach (var prop in activity.Baggage)
342364
{
@@ -346,7 +368,7 @@ private RequestTelemetry InitializeRequestTelemetry(HttpContext httpContext, Act
346368
}
347369
}
348370

349-
this.client.Initialize(requestTelemetry);
371+
this.client.InitializeInstrumentationKey(requestTelemetry);
350372

351373
requestTelemetry.Source = GetAppIdFromRequestHeader(httpContext.Request.Headers, requestTelemetry.Context.InstrumentationKey);
352374

@@ -490,10 +512,6 @@ private void SetW3CContext(IHeaderDictionary requestHeaders, Activity activity,
490512
InjectionGuardConstants.TraceParentHeaderMaxLength);
491513
activity.SetTraceparent(parentTraceParent);
492514
}
493-
else
494-
{
495-
activity.GenerateW3CContext();
496-
}
497515

498516
string[] traceStateValues = HttpHeadersUtilities.SafeGetCommaSeparatedHeaderValues(requestHeaders, W3C.W3CConstants.TraceStateHeader,
499517
InjectionGuardConstants.TraceStateHeaderMaxLength, InjectionGuardConstants.TraceStateMaxPairs);

‎src/Microsoft.ApplicationInsights.AspNetCore/DiagnosticListeners/Implementation/HttpHeadersUtilities.cs‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -65,36 +65,19 @@ internal static bool ContainsRequestContextKeyValue(IHeaderDictionary headers, s
6565
return !string.IsNullOrEmpty(GetHeaderKeyValue(headers, RequestResponseHeaders.RequestContextHeader, keyName));
6666
}
6767

68-
internal static void SetRequestContextKeyValue(HttpHeaders headers, string keyName, string keyValue)
69-
{
70-
SetHeaderKeyValue(headers, RequestResponseHeaders.RequestContextHeader, keyName, keyValue);
71-
}
72-
7368
internal static void SetRequestContextKeyValue(IHeaderDictionary headers, string keyName, string keyValue)
7469
{
7570
SetHeaderKeyValue(headers, RequestResponseHeaders.RequestContextHeader, keyName, keyValue);
7671
}
7772

78-
internal static void SetHeaderKeyValue(HttpHeaders headers, string headerName, string keyName, string keyValue)
79-
{
80-
if (headers == null)
81-
{
82-
throw new ArgumentNullException(nameof(headers));
83-
}
84-
85-
IEnumerable<string> headerValues = GetHeaderValues(headers, headerName);
86-
headers.Remove(headerName);
87-
headers.Add(headerName, HeadersUtilities.SetHeaderKeyValue(headerValues, keyName, keyValue));
88-
}
89-
9073
internal static void SetHeaderKeyValue(IHeaderDictionary headers, string headerName, string keyName, string keyValue)
9174
{
9275
if (headers == null)
9376
{
9477
throw new ArgumentNullException(nameof(headers));
9578
}
9679

97-
headers[headerName] = new StringValues(HeadersUtilities.SetHeaderKeyValue(headers[headerName].AsEnumerable(), keyName, keyValue).ToArray());
80+
headers[headerName] = HeadersUtilities.SetHeaderKeyValue(headers[headerName], keyName, keyValue);
9881
}
9982

10083
internal static string[] SafeGetCommaSeparatedHeaderValues(IHeaderDictionary headers, string headerName, int maxLength, int maxItems)

‎src/Microsoft.ApplicationInsights.AspNetCore/Extensions/HttpRequestExtensions.cs‎

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,26 +28,12 @@ public static Uri GetUri(this HttpRequest request)
2828
{
2929
throw new ArgumentException("Http request Scheme is not specified");
3030
}
31-
32-
string hostName = request.Host.HasValue ? request.Host.ToString() : UnknownHostName;
3331

34-
var builder = new StringBuilder();
35-
36-
builder.Append(request.Scheme)
37-
.Append("://")
38-
.Append(hostName);
39-
40-
if (true == request.Path.HasValue)
41-
{
42-
builder.Append(request.Path.Value);
43-
}
44-
45-
if (true == request.QueryString.HasValue)
46-
{
47-
builder.Append(request.QueryString);
48-
}
49-
50-
return new Uri(builder.ToString());
32+
return new Uri(string.Concat(request.Scheme,
33+
"://",
34+
request.Host.HasValue ? request.Host.Value : UnknownHostName,
35+
request.Path.HasValue ? request.Path.Value : "",
36+
request.QueryString.HasValue ? request.QueryString.Value : ""));
5137
}
5238
}
5339
}

‎src/Microsoft.ApplicationInsights.AspNetCore/TelemetryInitializers/AspNetCoreEnvironmentTelemetryInitializer.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ public AspNetCoreEnvironmentTelemetryInitializer(IHostingEnvironment environment
2323

2424
/// <inheritdoc />
2525
public void Initialize(ITelemetry telemetry)
26-
{
27-
if (environment != null && !telemetry.Context.GlobalProperties.ContainsKey(AspNetCoreEnvironmentPropertyName))
26+
{
27+
if (environment != null && !telemetry.Context.Properties.ContainsKey(AspNetCoreEnvironmentPropertyName))
2828
{
29-
telemetry.Context.GlobalProperties.Add(AspNetCoreEnvironmentPropertyName, environment.EnvironmentName);
29+
telemetry.Context.Properties.Add(AspNetCoreEnvironmentPropertyName, environment.EnvironmentName);
3030
}
3131
}
3232
}

‎test/Microsoft.ApplicationInsights.AspNetCore.Tests/HeadersUtilitiesTest.cs‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
namespace Microsoft.ApplicationInsights.AspNetCore.Tests
22
{
33
using System.Collections.Generic;
4-
using System.Linq;
54
using DiagnosticListeners;
65
using Xunit;
76

@@ -30,36 +29,35 @@ public void ShouldReturnNullWhenKeyNotExists()
3029
[Fact]
3130
public void ShouldReturnHeadersWhenNoHeaderValues()
3231
{
33-
IEnumerable<string> newHeaders = HeadersUtilities.SetHeaderKeyValue(null, "Key", "Value");
32+
string[] newHeaders = HeadersUtilities.SetHeaderKeyValue(null, "Key", "Value");
3433

3534
Assert.NotNull(newHeaders);
36-
Assert.Equal(1, newHeaders.Count());
37-
Assert.Equal("Key=Value", newHeaders.First());
35+
Assert.Single(newHeaders);
36+
Assert.Equal("Key=Value", newHeaders[0]);
3837
}
3938

4039
[Fact]
4140
public void ShouldAppendHeaders()
4241
{
43-
IEnumerable<string> existing = new List<string>() { "ExistKey=ExistValue" };
44-
IEnumerable<string> result = HeadersUtilities.SetHeaderKeyValue(existing, "NewKey", "NewValue");
42+
string[] existing = new string[] { "ExistKey=ExistValue" };
43+
string[] result = HeadersUtilities.SetHeaderKeyValue(existing, "NewKey", "NewValue");
4544

4645
Assert.NotNull(result);
47-
Assert.Equal(2, result.Count());
48-
Assert.NotNull(result.FirstOrDefault(headerValue => headerValue.Equals("ExistKey=ExistValue")));
49-
Assert.NotNull(result.FirstOrDefault(headerValue => headerValue.Equals("NewKey=NewValue")));
46+
Assert.Equal(2, result.Length);
47+
Assert.Equal("ExistKey=ExistValue", result[0]);
48+
Assert.Equal("NewKey=NewValue", result[1]);
5049
}
5150

5251
[Fact]
5352
public void ShouldUpdateExistingHeader()
5453
{
55-
IEnumerable<string> existing = new List<string>() { "ExistKey=ExistValue", "NoiseKey=NoiseValue" };
56-
IEnumerable<string> result = HeadersUtilities.SetHeaderKeyValue(existing, "ExistKey", "NewValue");
54+
string[] existing = new string[] { "ExistKey=ExistValue", "NoiseKey=NoiseValue" };
55+
string[] result = HeadersUtilities.SetHeaderKeyValue(existing, "ExistKey", "NewValue");
5756

5857
Assert.NotNull(result);
59-
Assert.Equal(2, result.Count());
60-
Assert.Null(result.FirstOrDefault(headerValue => headerValue.Equals("ExistKey=ExistValue")));
61-
Assert.NotNull(result.FirstOrDefault(headerValue => headerValue.Equals("ExistKey=NewValue")));
62-
Assert.NotNull(result.FirstOrDefault(headerValue => headerValue.Equals("NoiseKey=NoiseValue")));
58+
Assert.Equal(2, result.Length);
59+
Assert.Equal("ExistKey=NewValue", result[0]);
60+
Assert.Equal("NoiseKey=NoiseValue", result[1]);
6361
}
6462
}
6563
}

‎test/Microsoft.ApplicationInsights.AspNetCore.Tests/HostingDiagnosticListenerTest.cs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -836,10 +836,6 @@ public void OnBeginRequestWithW3CSupportAndNoHeadersIsTrackedCorrectly(bool isAs
836836

837837
var activityInitializedByW3CHeader = Activity.Current;
838838

839-
if (!isAspNetCore2)
840-
{
841-
Assert.Null(activityInitializedByW3CHeader.ParentId);
842-
}
843839
Assert.NotNull(activityInitializedByW3CHeader.GetTraceId());
844840
Assert.Equal(32, activityInitializedByW3CHeader.GetTraceId().Length);
845841
Assert.Equal(16, activityInitializedByW3CHeader.GetSpanId().Length);

‎test/Microsoft.ApplicationInsights.AspNetCore.Tests/TelemetryInitializers/AspNetCoreEnvironmentTelemetryInitializerTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@ public void InitializeDoesNotOverrideExistingProperty()
1919
{
2020
var initializer = new AspNetCoreEnvironmentTelemetryInitializer(new HostingEnvironment() { EnvironmentName = "Production"});
2121
var telemetry = new RequestTelemetry();
22-
telemetry.Context.GlobalProperties.Add("AspNetCoreEnvironment", "Development");
22+
telemetry.Context.Properties.Add("AspNetCoreEnvironment", "Development");
2323
initializer.Initialize(telemetry);
2424

25-
Assert.Equal("Development", telemetry.Context.GlobalProperties["AspNetCoreEnvironment"]);
25+
Assert.Equal("Development", telemetry.Context.Properties["AspNetCoreEnvironment"]);
2626
}
2727

2828
[Fact]
@@ -32,7 +32,7 @@ public void InitializeSetsCurrentEnvironmentNameToProperty()
3232
var telemetry = new RequestTelemetry();
3333
initializer.Initialize(telemetry);
3434

35-
Assert.Equal("Production", telemetry.Context.GlobalProperties["AspNetCoreEnvironment"]);
35+
Assert.Equal("Production", telemetry.Context.Properties["AspNetCoreEnvironment"]);
3636
}
3737
}
3838
}

0 commit comments

Comments
 (0)