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

Commit

Permalink
Merge pull request #971 from microsoft/lmolkova/samplingFlagAndFixes
Browse files Browse the repository at this point in the history 
Do not set sampledOut flag if activity is recorded and parentId fixes
  • Loading branch information
Liudmila Molkova authored Sep 9, 2019
2 parents d55edb2 + 15eae02 commit 59efae2
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 42 deletions.
40 changes: 27 additions & 13 deletions ...cationInsights.AspNetCore/DiagnosticListeners/Implementation/HostingDiagnosticListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ public void OnHttpRequestInStart(HttpContext httpContext)
bool traceParentPresent = false;
var headers = httpContext.Request.Headers;

// 3 posibilities when TelemetryConfiguration.EnableW3CCorrelation = true
// 3 possibilities when TelemetryConfiguration.EnableW3CCorrelation = true
// 1. No incoming headers. originalParentId will be null. Simply use the Activity as such.
// 2. Incoming Request-ID Headers. originalParentId will be request-id, but Activity ignores this for ID calculations.
// If incoming ID is W3C compatible, ignore current Activity. Create new one with parent set to incoming W3C compatible rootid.
Expand All @@ -201,13 +201,15 @@ public void OnHttpRequestInStart(HttpContext httpContext)
// 3a - 2.XX Need to ignore current Activity, and create new from incoming W3C TraceParent header.
// 3b - 3.XX Use Activity as such because 3.XX is W3C Aware.

// Another 3 posibilities when TelemetryConfiguration.EnableW3CCorrelation = false
// Another 3 possibilities when TelemetryConfiguration.EnableW3CCorrelation = false
// 1. No incoming headers. originalParentId will be null. Simply use the Activity as such.
// 2. Incoming Request-ID Headers. originalParentId will be request-id, Activity uses this for ID calculations.
// 3. Incoming TraceParent header. Will simply Ignore W3C headers, and Current Activity used as such.

// Attempt to find parent from incoming W3C Headers which 2.XX Hosting is unaware of.
if (this.aspNetCoreMajorVersion != AspNetCoreMajorVersion.Three && currentActivity.IdFormat == ActivityIdFormat.W3C && headers.TryGetValue(W3CConstants.TraceParentHeader, out StringValues traceParentValues)
if (this.aspNetCoreMajorVersion != AspNetCoreMajorVersion.Three
&& currentActivity.IdFormat == ActivityIdFormat.W3C
&& headers.TryGetValue(W3CConstants.TraceParentHeader, out StringValues traceParentValues)
&& traceParentValues != StringValues.Empty)
{
var parentTraceParent = StringUtilities.EnforceMaxLength(
Expand Down Expand Up @@ -282,7 +284,9 @@ public void OnHttpRequestInStart(HttpContext httpContext)
}

var requestTelemetry = this.InitializeRequestTelemetry(httpContext, currentActivity, Stopwatch.GetTimestamp(), legacyRootId);
requestTelemetry.Context.Operation.ParentId = originalParentId;

requestTelemetry.Context.Operation.ParentId =
GetParentId(currentActivity, originalParentId, requestTelemetry.Context.Operation.Id);

this.AddAppIdToResponseIfRequired(httpContext, requestTelemetry);
}
Expand Down Expand Up @@ -315,7 +319,6 @@ public void OnBeginRequest(HttpContext httpContext, long timestamp)

// 1.XX does not create Activity and SDK is responsible for creating Activity.
var activity = new Activity(ActivityCreatedByHostingDiagnosticListener);
string sourceAppId = null;
IHeaderDictionary requestHeaders = httpContext.Request.Headers;
string originalParentId = null;
string legacyRootId = null;
Expand All @@ -326,8 +329,8 @@ public void OnBeginRequest(HttpContext httpContext, long timestamp)
traceParentValues != StringValues.Empty)
{
var parentTraceParent = StringUtilities.EnforceMaxLength(traceParentValues.First(), InjectionGuardConstants.TraceParentHeaderMaxLength);
activity.SetParentId(parentTraceParent);
originalParentId = parentTraceParent;
activity.SetParentId(originalParentId);

ReadTraceState(requestHeaders, activity);
ReadCorrelationContext(requestHeaders, activity);
Expand Down Expand Up @@ -367,13 +370,9 @@ public void OnBeginRequest(HttpContext httpContext, long timestamp)
activity.Start();

var requestTelemetry = this.InitializeRequestTelemetry(httpContext, activity, timestamp, legacyRootId);
if (this.enableW3CHeaders && sourceAppId != null)
{
requestTelemetry.Source = sourceAppId;
}

// fix parent that may be modified by non-W3C operation correlation
requestTelemetry.Context.Operation.ParentId = originalParentId;
requestTelemetry.Context.Operation.ParentId =
GetParentId(activity, originalParentId, requestTelemetry.Context.Operation.Id);

this.AddAppIdToResponseIfRequired(httpContext, requestTelemetry);
}
Expand Down Expand Up @@ -546,6 +545,20 @@ public void OnCompleted()
{
}

private string GetParentId(Activity activity, string originalParentId, string operationId)
{
if (activity.IdFormat == ActivityIdFormat.W3C && activity.ParentSpanId != default)
{
var parentSpanId = activity.ParentSpanId.ToHexString();
if (parentSpanId != "0000000000000000")
{
return FormatTelemetryId(operationId, parentSpanId);
}
}

return originalParentId;
}

private static string ExtractOperationIdFromRequestId(string originalParentId)
{
if (originalParentId[0] == '|')
Expand Down Expand Up @@ -718,10 +731,11 @@ private RequestTelemetry InitializeRequestTelemetry(HttpContext httpContext, Act
{
requestTelemetry.Context.Operation.Id = activity.RootId;
requestTelemetry.Id = activity.Id;
AspNetCoreEventSource.Instance.RequestTelemetryCreated("Hierrarchical", requestTelemetry.Id, requestTelemetry.Context.Operation.Id);
AspNetCoreEventSource.Instance.RequestTelemetryCreated("Hierarchical", requestTelemetry.Id, requestTelemetry.Context.Operation.Id);
}

if (this.proactiveSamplingEnabled
&& !activity.Recorded
&& this.configuration != null
&& !string.IsNullOrEmpty(requestTelemetry.Context.Operation.Id)
&& SamplingScoreGenerator.GetSamplingScore(requestTelemetry.Context.Operation.Id) >= this.configuration.GetLastObservedSamplingPercentage(requestTelemetry.ItemTypeFlag))
Expand Down
81 changes: 58 additions & 23 deletions test/Microsoft.ApplicationInsights.AspNetCore.Tests/HostingDiagnosticListenerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,8 @@ public void RequestWithNonW3CCompatibleNonHierarchicalRequestIdCreateNewActivity
[InlineData(AspNetCoreMajorVersion.One, false)]
[InlineData(AspNetCoreMajorVersion.Two, false)]
[InlineData(AspNetCoreMajorVersion.Three, false)]
public void RequestWithW3CTraceParentCreateNewActivityAndPopulateRequestTelemetry(AspNetCoreMajorVersion aspNetCoreMajorVersion, bool IsW3C)
public void RequestWithW3CTraceParentCreateNewActivityAndPopulateRequestTelemetry(AspNetCoreMajorVersion aspNetCoreMajorVersion,
bool isW3C)
{
// Tests Request correlation when incoming request has only Request-ID headers.
HttpContext context = CreateContext(HttpRequestScheme, HttpRequestHost, "/Test", method: "POST");
Expand All @@ -469,7 +470,7 @@ public void RequestWithW3CTraceParentCreateNewActivityAndPopulateRequestTelemetr
context.Request.Headers[Microsoft.ApplicationInsights.W3C.W3CConstants.TraceStateHeader] = "w3cprop1=value1, w3cprop2=value2";
context.Request.Headers[RequestResponseHeaders.CorrelationContextHeader] = "prop1=value1, prop2=value2";

using (var hostingListener = CreateHostingListener(aspNetCoreMajorVersion, isW3C: IsW3C))
using (var hostingListener = CreateHostingListener(aspNetCoreMajorVersion, isW3C: isW3C))
{
HandleRequestBegin(hostingListener, context, 0, aspNetCoreMajorVersion);
var activity = Activity.Current;
Expand All @@ -479,7 +480,7 @@ public void RequestWithW3CTraceParentCreateNewActivityAndPopulateRequestTelemetr
{
Assert.Equal(ActivityCreatedByHostingDiagnosticListener, activity.OperationName);
}
else if (aspNetCoreMajorVersion == AspNetCoreMajorVersion.Two && IsW3C)
else if (aspNetCoreMajorVersion == AspNetCoreMajorVersion.Two && isW3C)
{
Assert.Equal(ActivityCreatedByHostingDiagnosticListener, activity.OperationName);
}
Expand All @@ -489,7 +490,7 @@ public void RequestWithW3CTraceParentCreateNewActivityAndPopulateRequestTelemetr
Assert.NotEqual(ActivityCreatedByHostingDiagnosticListener, activity.OperationName);
}

if (IsW3C)
if (isW3C)
{
Assert.Equal("w3cprop1=value1, w3cprop2=value2", activity.TraceStateString);
}
Expand All @@ -501,16 +502,16 @@ public void RequestWithW3CTraceParentCreateNewActivityAndPopulateRequestTelemetr
Assert.Single(sentTelemetry);
var requestTelemetry = (RequestTelemetry)this.sentTelemetry.Single();

if (IsW3C)
if (isW3C)
{
// parentid populated only in W3C mode
ValidateRequestTelemetry(requestTelemetry, activity, IsW3C, expectedParentId: traceParent, expectedSource: null);
ValidateRequestTelemetry(requestTelemetry, activity, true, expectedParentId: "|4e3083444c10254ba40513c7316332eb.e2a5f830c0ee2c46.", expectedSource: null);
Assert.Equal("value1", requestTelemetry.Properties["prop1"]);
Assert.Equal("value2", requestTelemetry.Properties["prop2"]);
}
else
{
ValidateRequestTelemetry(requestTelemetry, activity, IsW3C, expectedParentId: null, expectedSource: null);
ValidateRequestTelemetry(requestTelemetry, activity, false, expectedParentId: null, expectedSource: null);
}
}
}
Expand All @@ -522,7 +523,9 @@ public void RequestWithW3CTraceParentCreateNewActivityAndPopulateRequestTelemetr
[InlineData(AspNetCoreMajorVersion.One, false)]
[InlineData(AspNetCoreMajorVersion.Two, false)]
[InlineData(AspNetCoreMajorVersion.Three, false)]
public void RequestWithW3CTraceParentButInvalidEntryCreateNewActivityAndPopulateRequestTelemetry(AspNetCoreMajorVersion aspNetCoreMajorVersion, bool IsW3C)
public void RequestWithW3CTraceParentButInvalidEntryCreateNewActivityAndPopulateRequestTelemetry(
AspNetCoreMajorVersion aspNetCoreMajorVersion,
bool isW3C)
{
// Tests Request correlation when incoming request has only Request-ID headers.
HttpContext context = CreateContext(HttpRequestScheme, HttpRequestHost, "/Test", method: "POST");
Expand All @@ -531,14 +534,14 @@ public void RequestWithW3CTraceParentButInvalidEntryCreateNewActivityAndPopulate
context.Request.Headers[Microsoft.ApplicationInsights.W3C.W3CConstants.TraceParentHeader] = traceParent;
context.Request.Headers[Microsoft.ApplicationInsights.W3C.W3CConstants.TraceStateHeader] = "prop1=value1, prop2=value2";

using (var hostingListener = CreateHostingListener(aspNetCoreMajorVersion, isW3C: IsW3C))
using (var hostingListener = CreateHostingListener(aspNetCoreMajorVersion, isW3C: isW3C))
{
HandleRequestBegin(hostingListener, context, 0, aspNetCoreMajorVersion);
var activity = Activity.Current;
Assert.NotNull(activity);
Assert.NotEqual(traceParent, activity.Id);

if (IsW3C)
if (isW3C)
{
Assert.Equal("prop1=value1, prop2=value2", activity.TraceStateString);
}
Expand All @@ -550,14 +553,14 @@ public void RequestWithW3CTraceParentButInvalidEntryCreateNewActivityAndPopulate
Assert.Single(sentTelemetry);
var requestTelemetry = (RequestTelemetry)this.sentTelemetry.Single();

if (IsW3C)
if (isW3C)
{
// parentid populated only in W3C mode
ValidateRequestTelemetry(requestTelemetry, activity, IsW3C, expectedParentId: traceParent, expectedSource: null);
ValidateRequestTelemetry(requestTelemetry, activity, true, expectedParentId: traceParent, expectedSource: null);
}
else
{
ValidateRequestTelemetry(requestTelemetry, activity, IsW3C, expectedParentId: null, expectedSource: null);
ValidateRequestTelemetry(requestTelemetry, activity, false, expectedParentId: null, expectedSource: null);
}
}
}
Expand Down Expand Up @@ -598,7 +601,7 @@ public void RequestWithBothW3CAndRequestIdCreateNewActivityAndPopulateRequestTel
if (IsW3C)
{
Assert.Equal("4e3083444c10254ba40513c7316332eb", requestTelemetry.Context.Operation.Id);
ValidateRequestTelemetry(requestTelemetry, activity, IsW3C, expectedParentId: traceParent, expectedSource:null);
ValidateRequestTelemetry(requestTelemetry, activity, IsW3C, expectedParentId: "|4e3083444c10254ba40513c7316332eb.e2a5f830c0ee2c46.", expectedSource:null);
}
else
{
Expand All @@ -614,17 +617,22 @@ public void RequestWithBothW3CAndRequestIdCreateNewActivityAndPopulateRequestTel
[Theory]
[InlineData(true)]
[InlineData(false)]
public void OnHttpRequestInStartInitializeTelemetryIfActivityParentIdIsNotNull(bool IsW3C)
public void OnHttpRequestInStartInitializeTelemetryIfActivityParentIdIsNotNull(bool isW3C)
{
var context = CreateContext(HttpRequestScheme, HttpRequestHost, "/Test", method: "POST");
string parentId = "|8ee8641cbdd8dd280d239fa2121c7e4e.df07da90a5b27d93.";

var parent = "|8ee8641cbdd8dd280d239fa2121c7e4e.df07da90a5b27d93.";
context.Request.Headers[RequestResponseHeaders.RequestIdHeader] = parent;

Activity activity;
Activity activityBySDK;

using (var hostingListener = CreateHostingListener(AspNetCoreMajorVersion.Two, isW3C: IsW3C))
using (var hostingListener = CreateHostingListener(AspNetCoreMajorVersion.Two, isW3C: isW3C))
{
activity = new Activity("operation");
activity.SetParentId(parentId);

// pretend ASP.NET Core read it
activity.SetParentId(parent);
activity.AddBaggage("item1", "value1");
activity.AddBaggage("item2", "value2");

Expand All @@ -639,9 +647,8 @@ public void OnHttpRequestInStartInitializeTelemetryIfActivityParentIdIsNotNull(b
Assert.Single(sentTelemetry);
var requestTelemetry = this.sentTelemetry.First() as RequestTelemetry;

ValidateRequestTelemetry(requestTelemetry, activityBySDK, IsW3C, expectedParentId: parentId);
ValidateRequestTelemetry(requestTelemetry, activityBySDK, isW3C, expectedParentId: "|8ee8641cbdd8dd280d239fa2121c7e4e.df07da90a5b27d93.");
Assert.Equal("8ee8641cbdd8dd280d239fa2121c7e4e", requestTelemetry.Context.Operation.Id);
Assert.Equal(requestTelemetry.Context.Operation.ParentId, activity.ParentId);
Assert.Equal(requestTelemetry.Properties.Count, activity.Baggage.Count());

foreach (var prop in activity.Baggage)
Expand Down Expand Up @@ -1014,7 +1021,35 @@ public void RequestTelemetryIsProactivelySampledOutIfFeatureFlagIsOn(AspNetCoreM
[InlineData(AspNetCoreMajorVersion.One)]
[InlineData(AspNetCoreMajorVersion.Two)]
[InlineData(AspNetCoreMajorVersion.Three)]
public void RequestTelemetryIsNotProactivelySampledOutIfFeatureFlasIfOff(AspNetCoreMajorVersion aspNetCoreMajorVersion)
public void RequestTelemetryIsProactivelySampledInIfFeatureFlagIsOnButActivityIsRecorded(AspNetCoreMajorVersion aspNetCoreMajorVersion)
{
TelemetryConfiguration config = TelemetryConfiguration.CreateDefault();
config.ExperimentalFeatures.Add("proactiveSampling");
config.SetLastObservedSamplingPercentage(SamplingTelemetryItemTypes.Request, 0);

HttpContext context = CreateContext(HttpRequestScheme, HttpRequestHost, "/Test", method: "POST");
var traceParent = "00-4e3083444c10254ba40513c7316332eb-e2a5f830c0ee2c46-01";
context.Request.Headers[Microsoft.ApplicationInsights.W3C.W3CConstants.TraceParentHeader] = traceParent;

using (var hostingListener = CreateHostingListener(aspNetCoreMajorVersion, config, true))
{
HandleRequestBegin(hostingListener, context, 0, aspNetCoreMajorVersion);

Assert.NotNull(Activity.Current);
Assert.True(Activity.Current.Recorded);

var requestTelemetry = context.Features.Get<RequestTelemetry>();
Assert.NotNull(requestTelemetry);
Assert.False(requestTelemetry.IsSampledOutAtHead);
ValidateRequestTelemetry(requestTelemetry, Activity.Current, true, "|4e3083444c10254ba40513c7316332eb.e2a5f830c0ee2c46.");
}
}

[Theory]
[InlineData(AspNetCoreMajorVersion.One)]
[InlineData(AspNetCoreMajorVersion.Two)]
[InlineData(AspNetCoreMajorVersion.Three)]
public void RequestTelemetryIsNotProactivelySampledOutIfFeatureFlagIfOff(AspNetCoreMajorVersion aspNetCoreMajorVersion)
{
TelemetryConfiguration config = TelemetryConfiguration.CreateDefault();
config.SetLastObservedSamplingPercentage(SamplingTelemetryItemTypes.Request, 0);
Expand Down Expand Up @@ -1131,12 +1166,12 @@ private static string FormatTelemetryId(string traceId, string spanId)
return string.Concat("|", traceId, ".", spanId, ".");
}

private void ValidateRequestTelemetry(RequestTelemetry requestTelemetry, Activity activity, bool IsW3C, string expectedParentId = null, string expectedSource = null)
private void ValidateRequestTelemetry(RequestTelemetry requestTelemetry, Activity activity, bool isW3C, string expectedParentId = null, string expectedSource = null)
{
Assert.NotNull(requestTelemetry);
Assert.Equal(expectedParentId, requestTelemetry.Context.Operation.ParentId);
Assert.Equal(expectedSource, requestTelemetry.Source);
if (IsW3C)
if (isW3C)
{
Assert.Equal(requestTelemetry.Id, FormatTelemetryId(activity.TraceId.ToHexString(), activity.SpanId.ToHexString()));
Assert.Equal(requestTelemetry.Context.Operation.Id, activity.TraceId.ToHexString());
Expand Down
Loading

0 comments on commit 59efae2

Please sign in to comment.