From 55cb75976eeb333382cd3f9c587744033093a354 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 22 Nov 2021 15:32:47 -0800 Subject: [PATCH 1/8] void accessing Id on activity start --- .../AspNetTelemetryEventSource.cs | 8 +++++++- src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs index 9fa3a5ef871..3cfb44e40ca 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs @@ -37,7 +37,13 @@ public void ActivityStarted(Activity activity) { if (this.IsEnabled(EventLevel.Verbose, EventKeywords.All)) { - this.ActivityStarted(activity?.Id); + // Accessing activity.Id here will cause the Id to be initialized + // before the sampler runs. This will result in Id not reflecting the + // correct sampling flags + // https://github.com/dotnet/runtime/issues/61857 + var activityId = string.Concat("00-", activity.TraceId.ToHexString(), "-", activity.SpanId.ToHexString()); + activityId = string.Concat(activityId, (activity.ActivityTraceFlags & ActivityTraceFlags.Recorded) != 0 ? "-01" : "-00"); + this.ActivityStarted(activityId); } } diff --git a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs index 13ec409906e..45b1df20230 100644 --- a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs +++ b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs @@ -105,7 +105,13 @@ public void ActivityStarted(Activity activity) { if (this.IsEnabled(EventLevel.Verbose, EventKeywords.All)) { - this.ActivityStarted(activity.OperationName, activity.Id); + // Accessing activity.Id here will cause the Id to be initialized + // before the sampler runs. This will result in Id not reflecting the + // correct sampling flags + // https://github.com/dotnet/runtime/issues/61857 + var activityId = string.Concat("00-", activity.TraceId.ToHexString(), "-", activity.SpanId.ToHexString()); + activityId = string.Concat(activityId, (activity.ActivityTraceFlags & ActivityTraceFlags.Recorded) != 0 ? "-01" : "-00"); + this.ActivityStarted(activity.OperationName, activityId); } } From 9d8b10f686548ade911c5687a11e29659ad578c0 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 22 Nov 2021 15:41:53 -0800 Subject: [PATCH 2/8] null check --- .../AspNetTelemetryEventSource.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs index 3cfb44e40ca..548295f9973 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs @@ -41,9 +41,17 @@ public void ActivityStarted(Activity activity) // before the sampler runs. This will result in Id not reflecting the // correct sampling flags // https://github.com/dotnet/runtime/issues/61857 - var activityId = string.Concat("00-", activity.TraceId.ToHexString(), "-", activity.SpanId.ToHexString()); - activityId = string.Concat(activityId, (activity.ActivityTraceFlags & ActivityTraceFlags.Recorded) != 0 ? "-01" : "-00"); - this.ActivityStarted(activityId); + string activityId = null; + if (activity == null) + { + this.ActivityStarted(activityId); + } + else + { + activityId = string.Concat("00-", activity.TraceId.ToHexString(), "-", activity.SpanId.ToHexString()); + activityId = string.Concat(activityId, (activity.ActivityTraceFlags & ActivityTraceFlags.Recorded) != 0 ? "-01" : "-00"); + this.ActivityStarted(activityId); + } } } From 28de5b6290baa79f81acbba8e6ce246eb15f35b7 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 22 Nov 2021 15:59:34 -0800 Subject: [PATCH 3/8] commit suggestion Co-authored-by: Reiley Yang --- .../AspNetTelemetryEventSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs index 548295f9973..95f70b28116 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs @@ -37,7 +37,7 @@ public void ActivityStarted(Activity activity) { if (this.IsEnabled(EventLevel.Verbose, EventKeywords.All)) { - // Accessing activity.Id here will cause the Id to be initialized + // Accessing activity.Id here will cause the Id to be calculated and cached // before the sampler runs. This will result in Id not reflecting the // correct sampling flags // https://github.com/dotnet/runtime/issues/61857 From 702228a089b65079a8de652e5d78629338422e09 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 22 Nov 2021 15:59:49 -0800 Subject: [PATCH 4/8] commit suggestion Co-authored-by: Reiley Yang --- .../AspNetTelemetryEventSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs index 95f70b28116..5ec417c5b2c 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs @@ -39,7 +39,7 @@ public void ActivityStarted(Activity activity) { // Accessing activity.Id here will cause the Id to be calculated and cached // before the sampler runs. This will result in Id not reflecting the - // correct sampling flags + // correct sampling flags. // https://github.com/dotnet/runtime/issues/61857 string activityId = null; if (activity == null) From 4849453cd1a39587000d8b769ff5e2a9914a95fc Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 22 Nov 2021 17:40:14 -0800 Subject: [PATCH 5/8] revert back change in aspnet event source --- .../AspNetTelemetryEventSource.cs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs index 5ec417c5b2c..9fa3a5ef871 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/AspNetTelemetryEventSource.cs @@ -37,21 +37,7 @@ public void ActivityStarted(Activity activity) { if (this.IsEnabled(EventLevel.Verbose, EventKeywords.All)) { - // Accessing activity.Id here will cause the Id to be calculated and cached - // before the sampler runs. This will result in Id not reflecting the - // correct sampling flags. - // https://github.com/dotnet/runtime/issues/61857 - string activityId = null; - if (activity == null) - { - this.ActivityStarted(activityId); - } - else - { - activityId = string.Concat("00-", activity.TraceId.ToHexString(), "-", activity.SpanId.ToHexString()); - activityId = string.Concat(activityId, (activity.ActivityTraceFlags & ActivityTraceFlags.Recorded) != 0 ? "-01" : "-00"); - this.ActivityStarted(activityId); - } + this.ActivityStarted(activity?.Id); } } From 8cca7510cb1c5827f073450ebeb5a4bc266ab11f Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 22 Nov 2021 17:44:28 -0800 Subject: [PATCH 6/8] update comment --- src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs index 45b1df20230..02d9e91b09a 100644 --- a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs +++ b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs @@ -106,7 +106,8 @@ public void ActivityStarted(Activity activity) if (this.IsEnabled(EventLevel.Verbose, EventKeywords.All)) { // Accessing activity.Id here will cause the Id to be initialized - // before the sampler runs. This will result in Id not reflecting the + // before the sampler runs in case where the activity is created using legacy way + // i.e. new Activity("Operation name"). This will result in Id not reflecting the // correct sampling flags // https://github.com/dotnet/runtime/issues/61857 var activityId = string.Concat("00-", activity.TraceId.ToHexString(), "-", activity.SpanId.ToHexString()); From 147388ad31f709044d00ee3809cbc41bf8c29d8a Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 23 Nov 2021 11:31:32 -0800 Subject: [PATCH 7/8] use enum hasflag --- src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs index 02d9e91b09a..ee304b07bac 100644 --- a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs +++ b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs @@ -111,7 +111,7 @@ public void ActivityStarted(Activity activity) // correct sampling flags // https://github.com/dotnet/runtime/issues/61857 var activityId = string.Concat("00-", activity.TraceId.ToHexString(), "-", activity.SpanId.ToHexString()); - activityId = string.Concat(activityId, (activity.ActivityTraceFlags & ActivityTraceFlags.Recorded) != 0 ? "-01" : "-00"); + activityId = string.Concat(activityId, activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded) ? "-01" : "-00"); this.ActivityStarted(activity.OperationName, activityId); } } From 892000239857d782baacad07dd2d96675976e8d8 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 23 Nov 2021 11:36:46 -0800 Subject: [PATCH 8/8] update changelog --- src/OpenTelemetry/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 77e87c2a07e..502d17aed6f 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Prevent accessing activity Id before sampler runs in case of legacy + activities. + ([2659](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2659)) + * Added `ReadOnlyTagCollection` and expose `Tags` on `MetricPoint` instead of `Keys`+`Values` ([#2642](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2642))