From f9d423bce72de2cc09e8a5a63707c56750cd24b5 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Wed, 1 Jun 2022 21:27:10 -0700 Subject: [PATCH 1/6] Add isRemote chekc for context propagation --- src/OpenTelemetry/Trace/TracerProviderSdk.cs | 8 ++++---- test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry/Trace/TracerProviderSdk.cs b/src/OpenTelemetry/Trace/TracerProviderSdk.cs index 4f2a292e6e..9a509f8f23 100644 --- a/src/OpenTelemetry/Trace/TracerProviderSdk.cs +++ b/src/OpenTelemetry/Trace/TracerProviderSdk.cs @@ -199,7 +199,7 @@ internal TracerProviderSdk( else if (sampler is AlwaysOffSampler) { listener.Sample = (ref ActivityCreationOptions options) => - !Sdk.SuppressInstrumentation ? PropagateOrIgnoreData(options.Parent.TraceId) : ActivitySamplingResult.None; + !Sdk.SuppressInstrumentation ? PropagateOrIgnoreData(options.Parent.TraceId, options.Parent.IsRemote) : ActivitySamplingResult.None; this.getRequestedDataAction = this.RunGetRequestedDataAlwaysOffSampler; } else @@ -393,13 +393,13 @@ private static ActivitySamplingResult ComputeActivitySamplingResult( return activitySamplingResult; } - return PropagateOrIgnoreData(options.Parent.TraceId); + return PropagateOrIgnoreData(options.Parent.TraceId, options.Parent.IsRemote); } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static ActivitySamplingResult PropagateOrIgnoreData(ActivityTraceId traceId) + private static ActivitySamplingResult PropagateOrIgnoreData(ActivityTraceId traceId, bool isParentRemote) { - var isRootSpan = traceId == default; + var isRootSpan = traceId == default || isParentRemote; // If it is the root span select PropagationData so the trace ID is preserved // even if no activity of the trace is recorded (sampled per OpenTelemetry parlance). diff --git a/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs b/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs index c3f7d3b90c..a18a24df01 100644 --- a/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs +++ b/test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs @@ -222,7 +222,7 @@ public void TracerProviderSdkSamplerAttributesAreAppliedToActivity(SamplingDecis } } - [Fact(Skip = "https://github.com/open-telemetry/opentelemetry-dotnet/issues/3315")] + [Fact] public void TracerSdkSetsActivitySamplingResultAsPropagationWhenParentIsRemote() { var testSampler = new TestSampler(); From de0573f15578dd558853acb5cb4792309518119b Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Wed, 1 Jun 2022 21:34:35 -0700 Subject: [PATCH 2/6] 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 c11858637c..38d51f3ca2 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Included `isRemote` check in `PropagateOrIgnoreData` in `TracerProviderSDK` to + ensure context propagation in case of a remote parent. + ([#3329](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3329)) + ## 1.3.0-rc.2 Released 2022-June-1 From 95c59d1bf1616baa46b4e6119b3e75f0b0466c74 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Thu, 2 Jun 2022 14:58:00 -0700 Subject: [PATCH 3/6] resolve pr comments --- src/OpenTelemetry/CHANGELOG.md | 5 +++-- src/OpenTelemetry/Trace/TracerProviderSdk.cs | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 38d51f3ca2..760f5372a9 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,8 +2,9 @@ ## Unreleased -* Included `isRemote` check in `PropagateOrIgnoreData` in `TracerProviderSDK` to - ensure context propagation in case of a remote parent. +* `TracerProviderSDK` modified for spans with remote parent to maintain context + propagation. For such spans activity will be created irrespective of + SamplingResult. ([#3329](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3329)) ## 1.3.0-rc.2 diff --git a/src/OpenTelemetry/Trace/TracerProviderSdk.cs b/src/OpenTelemetry/Trace/TracerProviderSdk.cs index 9a509f8f23..a4ff2d5b7c 100644 --- a/src/OpenTelemetry/Trace/TracerProviderSdk.cs +++ b/src/OpenTelemetry/Trace/TracerProviderSdk.cs @@ -399,11 +399,11 @@ private static ActivitySamplingResult ComputeActivitySamplingResult( [MethodImpl(MethodImplOptions.AggressiveInlining)] private static ActivitySamplingResult PropagateOrIgnoreData(ActivityTraceId traceId, bool isParentRemote) { - var isRootSpan = traceId == default || isParentRemote; + var isRootSpan = traceId == default; - // If it is the root span select PropagationData so the trace ID is preserved + // If it is the root span or the parent is remote select PropagationData so the trace ID is preserved // even if no activity of the trace is recorded (sampled per OpenTelemetry parlance). - return isRootSpan + return (isRootSpan || isParentRemote) ? ActivitySamplingResult.PropagationData : ActivitySamplingResult.None; } From 43f8de6ef0fe151e874abe258bec2c5132d1eff6 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Thu, 2 Jun 2022 14:59:55 -0700 Subject: [PATCH 4/6] reword chagelog --- src/OpenTelemetry/CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 760f5372a9..9be1f4f6b8 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,9 +2,9 @@ ## Unreleased -* `TracerProviderSDK` modified for spans with remote parent to maintain context - propagation. For such spans activity will be created irrespective of - SamplingResult. +* `TracerProviderSDK` modified for spans with remote parent. For such spans + activity will be created irrespective of SamplingResult, to maintain context + propagation. ([#3329](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3329)) ## 1.3.0-rc.2 From c9880ccbeb2368b3a994e76177dff3a22935f242 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 6 Jun 2022 10:30:29 -0700 Subject: [PATCH 5/6] refactor for minor perf --- src/OpenTelemetry/Trace/TracerProviderSdk.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry/Trace/TracerProviderSdk.cs b/src/OpenTelemetry/Trace/TracerProviderSdk.cs index a4ff2d5b7c..2bd6f65ac1 100644 --- a/src/OpenTelemetry/Trace/TracerProviderSdk.cs +++ b/src/OpenTelemetry/Trace/TracerProviderSdk.cs @@ -199,7 +199,7 @@ internal TracerProviderSdk( else if (sampler is AlwaysOffSampler) { listener.Sample = (ref ActivityCreationOptions options) => - !Sdk.SuppressInstrumentation ? PropagateOrIgnoreData(options.Parent.TraceId, options.Parent.IsRemote) : ActivitySamplingResult.None; + !Sdk.SuppressInstrumentation ? PropagateOrIgnoreData(options.Parent) : ActivitySamplingResult.None; this.getRequestedDataAction = this.RunGetRequestedDataAlwaysOffSampler; } else @@ -393,17 +393,17 @@ private static ActivitySamplingResult ComputeActivitySamplingResult( return activitySamplingResult; } - return PropagateOrIgnoreData(options.Parent.TraceId, options.Parent.IsRemote); + return PropagateOrIgnoreData(options.Parent); } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static ActivitySamplingResult PropagateOrIgnoreData(ActivityTraceId traceId, bool isParentRemote) + private static ActivitySamplingResult PropagateOrIgnoreData(in ActivityContext context) { - var isRootSpan = traceId == default; + var isRootSpan = context.TraceId == default; // If it is the root span or the parent is remote select PropagationData so the trace ID is preserved // even if no activity of the trace is recorded (sampled per OpenTelemetry parlance). - return (isRootSpan || isParentRemote) + return (isRootSpan || context.IsRemote) ? ActivitySamplingResult.PropagationData : ActivitySamplingResult.None; } From fb2d5a6e8c8c73f008589e2346d036276a40a138 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 6 Jun 2022 10:33:30 -0700 Subject: [PATCH 6/6] minor --- src/OpenTelemetry/Trace/TracerProviderSdk.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry/Trace/TracerProviderSdk.cs b/src/OpenTelemetry/Trace/TracerProviderSdk.cs index 2bd6f65ac1..cd760e2954 100644 --- a/src/OpenTelemetry/Trace/TracerProviderSdk.cs +++ b/src/OpenTelemetry/Trace/TracerProviderSdk.cs @@ -397,13 +397,13 @@ private static ActivitySamplingResult ComputeActivitySamplingResult( } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static ActivitySamplingResult PropagateOrIgnoreData(in ActivityContext context) + private static ActivitySamplingResult PropagateOrIgnoreData(in ActivityContext parentContext) { - var isRootSpan = context.TraceId == default; + var isRootSpan = parentContext.TraceId == default; // If it is the root span or the parent is remote select PropagationData so the trace ID is preserved // even if no activity of the trace is recorded (sampled per OpenTelemetry parlance). - return (isRootSpan || context.IsRemote) + return (isRootSpan || parentContext.IsRemote) ? ActivitySamplingResult.PropagationData : ActivitySamplingResult.None; }