Merge pull request #165 from microsoft/andrueastman/FixActivitySources

Adds registry for activity sources
microsoft · Nov 10, 2023 · f0e7768 · f0e7768
2 parents ccb5057 + 1657eaa
commit f0e7768
Show file tree

Hide file tree

Showing 13 changed files with 109 additions and 40 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [1.3.1] - 2023-11-10
+
+### Added
+
+- Fixes multiple initialization of `ActivitySource` instances on each request send(https://github.com/microsoft/kiota-http-dotnet/issues/161).
+
 ## [1.3.0] - 2023-11-02
 
 ### Added

diff --git a/Microsoft.Kiota.Http.HttpClientLibrary.Tests/Middleware/ActivitySourceRegistryTests.cs b/Microsoft.Kiota.Http.HttpClientLibrary.Tests/Middleware/ActivitySourceRegistryTests.cs
@@ -0,0 +1,48 @@
+using System;
+using Microsoft.Kiota.Http.HttpClientLibrary.Middleware;
+using Xunit;
+
+namespace Microsoft.Kiota.Http.HttpClientLibrary.Tests.Middleware.Registries
+{
+    public class ActivitySourceRegistryTests
+    {
+        [Fact]
+        public void Defensive()
+        {
+            Assert.Throws<ArgumentNullException>(() => ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource(""));
+            Assert.Throws<ArgumentNullException>(() => ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource(null));
+        }
+
+        [Fact]
+        public void CreatesNewInstanceOnFirstCallAndReturnsSameInstance()
+        {
+            // Act
+            var activitySource = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource("sample source");
+            Assert.NotNull(activitySource);
+
+            var activitySource2 = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource("sample source");
+            Assert.NotNull(activitySource);
+
+            // They are the same instance
+            Assert.Equal(activitySource, activitySource2);
+            Assert.Equal("sample source", activitySource.Name);
+            Assert.Equal("sample source", activitySource2.Name);
+        }
+
+        [Fact]
+        public void CreatesDifferentInstances()
+        {
+            // Act
+            var activitySource = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource("sample source");
+            Assert.NotNull(activitySource);
+            Assert.Equal("sample source", activitySource.Name);
+
+            var activitySource2 = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource("sample source 2");
+            Assert.NotNull(activitySource);
+            Assert.Equal("sample source 2", activitySource2.Name);
+
+            // They are not the same instance
+            Assert.NotEqual(activitySource, activitySource2);
+        }
+    }
+}
diff --git a/src/KiotaClientFactory.cs b/src/KiotaClientFactory.cs
@@ -65,7 +65,7 @@ public static IList<DelegatingHandler> CreateDefaultHandlers()
             }
             if(finalHandler != null)
                 handlers[handlers.Length-1].InnerHandler = finalHandler;
-            return handlers.First();
+            return handlers[0];//first
         }
         /// <summary>
         /// Creates a <see cref="DelegatingHandler"/> to use for the <see cref="HttpClient" /> from the provided <see cref="DelegatingHandler"/> instances. Order matters.

diff --git a/src/Microsoft.Kiota.Http.HttpClientLibrary.csproj b/src/Microsoft.Kiota.Http.HttpClientLibrary.csproj
@@ -14,7 +14,7 @@
     <PackageProjectUrl>https://aka.ms/kiota/docs</PackageProjectUrl>
     <EmbedUntrackedSources>true</EmbedUntrackedSources>
     <Deterministic>true</Deterministic>
-    <VersionPrefix>1.3.0</VersionPrefix>
+    <VersionPrefix>1.3.1</VersionPrefix>
     <VersionSuffix></VersionSuffix>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
     <!-- Enable this line once we go live to prevent breaking changes -->

diff --git a/src/Middleware/ActivitySourceRegistry.cs b/src/Middleware/ActivitySourceRegistry.cs
@@ -0,0 +1,37 @@
+// ------------------------------------------------------------------------------
+//  Copyright (c) Microsoft Corporation.  All Rights Reserved.  Licensed under the MIT License.  See License in the project root for license information.
+// ------------------------------------------------------------------------------
+
+using System;
+using System.Collections.Concurrent;
+using System.Diagnostics;
+
+namespace Microsoft.Kiota.Http.HttpClientLibrary.Middleware;
+
+/// <summary>
+/// Internal static registry for activity sources during tracing.
+/// </summary>
+internal class ActivitySourceRegistry
+{
+    private readonly ConcurrentDictionary<string, ActivitySource> _activitySources = new (StringComparer.OrdinalIgnoreCase);
+
+    /// <summary>
+    /// The default instance of the registry
+    /// </summary>
+    public static readonly ActivitySourceRegistry DefaultInstance = new();
+
+    /// <summary>
+    /// Get an a <see cref="ActivitySource"/> instance with the given name or create one.
+    /// </summary>
+    /// <param name="sourceName">The name of the <see cref="ActivitySource"/></param>
+    /// <returns></returns>
+    /// <exception cref="ArgumentNullException">When the parameter is null or empty</exception>
+    public ActivitySource GetOrCreateActivitySource(string sourceName)
+    {
+        if(string.IsNullOrEmpty(sourceName))
+            throw new ArgumentNullException(nameof(sourceName));
+
+        return _activitySources.GetOrAdd(sourceName, static source => new ActivitySource(source));
+
+    }
+}
diff --git a/src/Middleware/ChaosHandler.cs b/src/Middleware/ChaosHandler.cs
@@ -56,18 +56,16 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
 
             // Select global or per request options
             var chaosHandlerOptions = request.GetRequestOption<ChaosHandlerOption>() ?? _chaosHandlerOptions;
-            ActivitySource? activitySource;
             Activity? activity;
-            if(request.GetRequestOption<ObservabilityOptions>() is ObservabilityOptions obsOptions)
+            if(request.GetRequestOption<ObservabilityOptions>() is { } obsOptions)
             {
-                activitySource = new ActivitySource(obsOptions.TracerInstrumentationName);
+                var activitySource = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource(obsOptions.TracerInstrumentationName);
                 activity = activitySource?.StartActivity($"{nameof(ChaosHandler)}_{nameof(SendAsync)}");
                 activity?.SetTag("com.microsoft.kiota.handler.chaos.enable", true);
             }
             else
             {
                 activity = null;
-                activitySource = null;
             }
             try
             {
@@ -94,7 +92,6 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
             finally
             {
                 activity?.Dispose();
-                activitySource?.Dispose();
             }
         }
 

diff --git a/src/Middleware/CompressionHandler.cs b/src/Middleware/CompressionHandler.cs
@@ -31,18 +31,16 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
             if(request == null)
                 throw new ArgumentNullException(nameof(request));
 
-            ActivitySource? activitySource;
             Activity? activity;
-            if(request.GetRequestOption<ObservabilityOptions>() is ObservabilityOptions obsOptions)
+            if(request.GetRequestOption<ObservabilityOptions>() is { } obsOptions)
             {
-                activitySource = new ActivitySource(obsOptions.TracerInstrumentationName);
+                var activitySource = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource(obsOptions.TracerInstrumentationName);
                 activity = activitySource?.StartActivity($"{nameof(CompressionHandler)}_{nameof(SendAsync)}");
                 activity?.SetTag("com.microsoft.kiota.handler.compression.enable", true);
             }
             else
             {
                 activity = null;
-                activitySource = null;
             }
 
             try
@@ -79,7 +77,6 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
             finally
             {
                 activity?.Dispose();
-                activitySource?.Dispose();
             }
         }
 

diff --git a/src/Middleware/HeadersInspectionHandler.cs b/src/Middleware/HeadersInspectionHandler.cs
@@ -34,18 +34,16 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
 
         var options = request.GetRequestOption<HeadersInspectionHandlerOption>() ?? _defaultOptions;
 
-        ActivitySource? activitySource;
         Activity? activity;
-        if(request.GetRequestOption<ObservabilityOptions>() is ObservabilityOptions obsOptions)
+        if(request.GetRequestOption<ObservabilityOptions>() is { } obsOptions)
         {
-            activitySource = new ActivitySource(obsOptions.TracerInstrumentationName);
+            var activitySource = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource(obsOptions.TracerInstrumentationName);
             activity = activitySource?.StartActivity($"{nameof(RedirectHandler)}_{nameof(SendAsync)}");
             activity?.SetTag("com.microsoft.kiota.handler.headersInspection.enable", true);
         }
         else
         {
             activity = null;
-            activitySource = null;
         }
         try
         {
@@ -79,7 +77,6 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
         finally
         {
             activity?.Dispose();
-            activitySource?.Dispose();
         }
 
     }

diff --git a/src/Middleware/ParametersNameDecodingHandler.cs b/src/Middleware/ParametersNameDecodingHandler.cs
@@ -3,7 +3,6 @@
 // ------------------------------------------------------------------------------
 
 using System;
-using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
 using System.Net.Http;
@@ -35,15 +34,13 @@ public ParametersNameDecodingHandler(ParametersNameDecodingOption? options = def
     protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
     {
         var options = request.GetRequestOption<ParametersNameDecodingOption>() ?? EncodingOptions;
-        ActivitySource? activitySource;
         Activity? activity;
-        if (request.GetRequestOption<ObservabilityOptions>() is ObservabilityOptions obsOptions) {
-            activitySource = new ActivitySource(obsOptions.TracerInstrumentationName);
+        if (request.GetRequestOption<ObservabilityOptions>() is { } obsOptions) {
+            var activitySource = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource(obsOptions.TracerInstrumentationName);
             activity = activitySource.StartActivity($"{nameof(ParametersNameDecodingHandler)}_{nameof(SendAsync)}");
             activity?.SetTag("com.microsoft.kiota.handler.parameters_name_decoding.enable", true);
         } else {
             activity = null;
-            activitySource = null;
         }
         try {
             if(!request.RequestUri!.Query.Contains('%') ||
@@ -60,7 +57,6 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques
             return base.SendAsync(request, cancellationToken);
         } finally {
             activity?.Dispose();
-            activitySource?.Dispose();
         }
     }
     internal static string? DecodeUriEncodedString(string? original, char[] charactersToDecode) {

diff --git a/src/Middleware/RedirectHandler.cs b/src/Middleware/RedirectHandler.cs
@@ -49,9 +49,9 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
 
             ActivitySource? activitySource;
             Activity? activity;
-            if(request.GetRequestOption<ObservabilityOptions>() is ObservabilityOptions obsOptions)
+            if(request.GetRequestOption<ObservabilityOptions>() is { } obsOptions)
             {
-                activitySource = new ActivitySource(obsOptions.TracerInstrumentationName);
+                activitySource = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource(obsOptions.TracerInstrumentationName);
                 activity = activitySource?.StartActivity($"{nameof(RedirectHandler)}_{nameof(SendAsync)}");
                 activity?.SetTag("com.microsoft.kiota.handler.redirect.enable", true);
             }
@@ -157,7 +157,6 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
             finally
             {
                 activity?.Dispose();
-                activitySource?.Dispose();
             }
         }
 

diff --git a/src/Middleware/RetryHandler.cs b/src/Middleware/RetryHandler.cs
@@ -55,9 +55,9 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
             var retryOption = request.GetRequestOption<RetryHandlerOption>() ?? RetryOption;
             ActivitySource? activitySource;
             Activity? activity;
-            if(request.GetRequestOption<ObservabilityOptions>() is ObservabilityOptions obsOptions)
+            if(request.GetRequestOption<ObservabilityOptions>() is { } obsOptions)
             {
-                activitySource = new ActivitySource(obsOptions.TracerInstrumentationName);
+                activitySource = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource(obsOptions.TracerInstrumentationName);
                 activity = activitySource?.StartActivity($"{nameof(RetryHandler)}_{nameof(SendAsync)}");
                 activity?.SetTag("com.microsoft.kiota.handler.retry.enable", true);
             }
@@ -83,7 +83,6 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
             finally
             {
                 activity?.Dispose();
-                activitySource?.Dispose();
             }
         }
 

diff --git a/src/Middleware/UriReplacementHandler.cs b/src/Middleware/UriReplacementHandler.cs
@@ -1,7 +1,6 @@
 using System.Diagnostics;
 using System.Net.Http;
 using System.Threading.Tasks;
-using Microsoft.Kiota.Abstractions;
 using Microsoft.Kiota.Http.HttpClientLibrary.Extensions;
 using Microsoft.Kiota.Http.HttpClientLibrary.Middleware.Options;
 
@@ -28,18 +27,16 @@ public UriReplacementHandler(TUriReplacementHandlerOption uriReplacement)
     protected override async Task<HttpResponseMessage> SendAsync(
         HttpRequestMessage request, System.Threading.CancellationToken cancellationToken)
     {
-        ActivitySource? activitySource;
         Activity? activity;
-        if(request.GetRequestOption<ObservabilityOptions>() is ObservabilityOptions obsOptions)
+        if(request.GetRequestOption<ObservabilityOptions>() is { } obsOptions)
         {
-            activitySource = new ActivitySource(obsOptions.TracerInstrumentationName);
+            var activitySource = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource(obsOptions.TracerInstrumentationName);
             activity = activitySource.StartActivity($"{nameof(UriReplacementHandler<TUriReplacementHandlerOption>)}_{nameof(SendAsync)}");
             activity?.SetTag("com.microsoft.kiota.handler.uri_replacement.enable", uriReplacement.IsEnabled());
         }
         else
         {
             activity = null;
-            activitySource = null;
         }
 
         try
@@ -50,7 +47,6 @@ protected override async Task<HttpResponseMessage> SendAsync(
         finally
         {
             activity?.Dispose();
-            activitySource?.Dispose();
         }
     }
 }
diff --git a/src/Middleware/UserAgentHandler.cs b/src/Middleware/UserAgentHandler.cs
@@ -30,15 +30,13 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques
             if(request == null)
                 throw new ArgumentNullException(nameof(request));
 
-            ActivitySource? activitySource;
             Activity? activity;
-            if (request.GetRequestOption<ObservabilityOptions>() is ObservabilityOptions obsOptions) {
-                activitySource = new ActivitySource(obsOptions.TracerInstrumentationName);
+            if (request.GetRequestOption<ObservabilityOptions>() is { } obsOptions) {
+                var activitySource = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource(obsOptions.TracerInstrumentationName);
                 activity = activitySource?.StartActivity($"{nameof(UserAgentHandler)}_{nameof(SendAsync)}");
                 activity?.SetTag("com.microsoft.kiota.handler.useragent.enable", true);
             } else {
                 activity = null;
-                activitySource = null;
             }
             try {
                 var userAgentHandlerOption = request.GetRequestOption<UserAgentHandlerOption>() ?? _userAgentOption;
@@ -51,7 +49,6 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques
                 return base.SendAsync(request, cancellationToken);
             } finally {
                 activity?.Dispose();
-                activitySource?.Dispose();
             }
         }
     }