From c55145b186186634f92194ea9a1984e6e49f7388 Mon Sep 17 00:00:00 2001
From: Eddy Nakamura <ednakamu@microsoft.com>
Date: Tue, 3 Nov 2020 22:21:06 -0300
Subject: [PATCH] Fixing activityContext propagation (#1446)

* Fixing activityContext propagation

* Adding tests

* updating tests

* adding comments

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
---
 .../HttpWebRequestActivitySource.netfx.cs     |  8 ++--
 .../HttpWebRequestTests.Basic.netfx.cs        | 42 +++++++++++++++++++
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
index 139747c10..7603cf37a 100644
--- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
+++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
@@ -94,8 +94,6 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation
         {
             activity.DisplayName = HttpTagHelper.GetOperationNameForHttpMethod(request.Method);
 
-            InstrumentRequest(request, activity);
-
             if (activity.IsAllDataRequested)
             {
                 try
@@ -197,8 +195,8 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private static void InstrumentRequest(HttpWebRequest request, Activity activity)
-            => Options.Propagator.Inject(new PropagationContext(activity.Context, Baggage.Current), request, HttpWebRequestHeaderValuesSetter);
+        private static void InstrumentRequest(HttpWebRequest request, ActivityContext activityContext)
+            => Options.Propagator.Inject(new PropagationContext(activityContext, Baggage.Current), request, HttpWebRequestHeaderValuesSetter);
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private static bool IsRequestInstrumented(HttpWebRequest request)
@@ -214,7 +212,9 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation
             }
 
             var activity = WebRequestActivitySource.StartActivity(ActivityName, ActivityKind.Client);
+            var activityContext = Activity.Current?.Context ?? default;
 
+            InstrumentRequest(request, activityContext);
             if (activity == null)
             {
                 // There is a listener but it decided not to sample the current request.
diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.netfx.cs
index df5f86796..3e14632d6 100644
--- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.netfx.cs
+++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.netfx.cs
@@ -95,6 +95,48 @@ namespace OpenTelemetry.Instrumentation.Http.Tests
             parent.Stop();
         }
 
+        [Fact]
+        public async Task HttpWebRequestInstrumentationInjectsHeadersAsyncWhenActivityIsNotRecorded()
+        {
+            ActivityContext contentFromPropagator = default;
+            var activityProcessor = new Mock<BaseProcessor<Activity>>();
+            var propagator = new Mock<TextMapPropagator>();
+            propagator.Setup(m => m.Inject(It.IsAny<PropagationContext>(), It.IsAny<HttpWebRequest>(), It.IsAny<Action<HttpWebRequest, string, string>>()))
+                .Callback<PropagationContext, HttpWebRequest, Action<HttpWebRequest, string, string>>((context, message, action) =>
+                {
+                    contentFromPropagator = context.ActivityContext;
+                });
+
+            // Sdk.SetDefaultTextMapPropagator(propagator.Object);
+            using var shutdownSignal = Sdk.CreateTracerProviderBuilder()
+                .AddProcessor(activityProcessor.Object)
+                .AddHttpWebRequestInstrumentation(options => options.Propagator = propagator.Object)
+                .Build();
+
+            var request = (HttpWebRequest)WebRequest.Create(this.url);
+
+            request.Method = "GET";
+
+            var parent = new Activity("parent")
+                .SetIdFormat(ActivityIdFormat.W3C)
+                .Start();
+            parent.TraceStateString = "k1=v1,k2=v2";
+            parent.ActivityTraceFlags = ActivityTraceFlags.None;
+
+            using var response = await request.GetResponseAsync();
+
+            // By default parentbasedsampler is used.
+            // In this case, the parent is the manually created parentactivity, which will have TraceFlags as None.
+            // This causes child to be not created.
+            Assert.Empty(activityProcessor.Invocations);
+
+            Assert.Equal(parent.TraceId, contentFromPropagator.TraceId);
+            Assert.Equal(parent.SpanId, contentFromPropagator.SpanId);
+            Assert.NotEqual(default, contentFromPropagator.SpanId);
+
+            parent.Stop();
+        }
+
         [Fact]
         public async Task HttpWebRequestInstrumentationInjectsHeadersAsync_CustomFormat()
         {