Update changelog and add tests

open-telemetry · Oct 11, 2023 · 9717e8a · 9717e8a
1 parent 488d2ac
commit 9717e8a
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 0 deletions.
diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Fixed an issue where activities were closed incorrectly before processing completed.
+  Activity processor's `OnEnd` will now happen after `AspNetInstrumentationOptions.Enrich`.
+  ([#1388](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1388))
+
 * Update `OpenTelemetry.Api` to `1.6.0`.
   ([#1344](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1344))
 

diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+* Release together with `OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule`.
+  Fixed an issue where activities were closed incorrectly before processing completed.
+  Activity processor's `OnEnd` will now happen after `AspNetInstrumentationOptions.Enrich`.
+  ([#1388](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1388))
+
 ## 1.0.0-rc9.9
 
 Released 2023-Jun-09

diff --git a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs
@@ -447,6 +447,45 @@ public void Fire_Exception_Events()
         Assert.Equal(1, callbacksFired);
     }
 
+    [Fact]
+    public void Should_Handle_Activity_Events_In_Correct_Order()
+    {
+        var eventOrder = new List<string>();
+        const string ActivityOnStarted = "ActivityOnStarted";
+        const string ActivityOnStopped = "ActivityOnStarted";
+        const string OnStartCallback = "OnStartCallback";
+        const string OnStopCallback = "OnStopCallback";
+
+        this.EnableListener(_ => eventOrder.Add(ActivityOnStarted), _ => eventOrder.Add(ActivityOnStopped));
+
+        var context = HttpContextHelper.GetFakeHttpContext();
+        using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, (_, _) => eventOrder.Add(OnStartCallback));
+        ActivityHelper.StopAspNetActivity(this.noopTextMapPropagator, rootActivity, context, (_, _) => eventOrder.Add(OnStopCallback));
+
+        var expectedOrder = new List<string>()
+        {
+            ActivityOnStarted,
+            OnStartCallback,
+            OnStopCallback,
+            ActivityOnStopped,
+        };
+
+        Assert.Equal(expectedOrder, eventOrder);
+    }
+
+    [Fact]
+    public void Should_Not_Pass_Stopped_Activity_To_Callbacks()
+    {
+        this.EnableListener();
+
+        var wasStopped = false;
+        var context = HttpContextHelper.GetFakeHttpContext();
+        using var rootActivity = ActivityHelper.StartAspNetActivity(this.noopTextMapPropagator, context, (activity, _) => wasStopped = activity.IsStopped || wasStopped);
+        ActivityHelper.StopAspNetActivity(this.noopTextMapPropagator, rootActivity, context, (activity, _) => wasStopped = activity.IsStopped || wasStopped);
+
+        Assert.False(wasStopped);
+    }
+
     private void EnableListener(Action<Activity>? onStarted = null, Action<Activity>? onStopped = null, Func<ActivityContext, ActivitySamplingResult>? onSample = null)
     {
         Debug.Assert(this.activitySourceListener == null, "Cannot attach multiple listeners in tests.");