[sdk] Make ResourceBuilder.AddDetector factory pattern public (attemp…

…t 2) (#4261) Co-authored-by: Cijo Thomas <cijo.thomas@gmail.com>
open-telemetry · Mar 6, 2023 · a318130 · a318130
1 parent 1a30646
commit a318130
Show file tree

Hide file tree

Showing 12 changed files with 64 additions and 17 deletions.
diff --git a/src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net462/PublicAPI.Unshipped.txt
@@ -23,3 +23,4 @@ static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetExemplarFilter(th
 ~override OpenTelemetry.Metrics.AlwaysOnExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
 ~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(double value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
 ~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
+OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider!, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
diff --git a/src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net6.0/PublicAPI.Unshipped.txt
@@ -23,3 +23,4 @@ static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetExemplarFilter(th
 ~override OpenTelemetry.Metrics.AlwaysOnExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
 ~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(double value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
 ~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
+OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider!, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
diff --git a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt
@@ -23,3 +23,4 @@ static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetExemplarFilter(th
 ~override OpenTelemetry.Metrics.AlwaysOnExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
 ~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(double value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
 ~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
+OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider!, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
diff --git a/src/OpenTelemetry/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt
@@ -23,3 +23,4 @@ static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.SetExemplarFilter(th
 ~override OpenTelemetry.Metrics.AlwaysOnExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
 ~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(double value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
 ~override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string, object>> tags) -> bool
+OpenTelemetry.Resources.ResourceBuilder.AddDetector(System.Func<System.IServiceProvider!, OpenTelemetry.Resources.IResourceDetector!>! resourceDetectorFactory) -> OpenTelemetry.Resources.ResourceBuilder!
diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md
@@ -5,6 +5,9 @@
 * Added Exemplar support.
   ([#4217](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4217))
 
+* Added `AddDetector` factory overload on `ResourceBuilder`.
+  ([#4261](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4261))
+
 ## 1.4.0
 
 Released 2023-Feb-24

diff --git a/src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs b/src/OpenTelemetry/Logs/OpenTelemetryLoggerProvider.cs
@@ -80,6 +80,8 @@ internal OpenTelemetryLoggerProvider(OpenTelemetryLoggerOptions options, IServic
 
             Guard.ThrowIfNull(options);
 
+            this.ServiceProvider = serviceProvider;
+
             this.IncludeScopes = options.IncludeScopes;
             this.IncludeFormattedMessage = options.IncludeFormattedMessage;
             this.ParseStateValues = options.ParseStateValues;
@@ -96,6 +98,8 @@ internal OpenTelemetryLoggerProvider(OpenTelemetryLoggerOptions options, IServic
             OpenTelemetrySdkEventSource.Log.OpenTelemetryLoggerProviderEvent("OpenTelemetryLoggerProvider built successfully.");
         }
 
+        internal IServiceProvider? ServiceProvider { get; }
+
         internal IExternalScopeProvider? ScopeProvider { get; private set; }
 
         internal ILogRecordPool LogRecordPool => this.threadStaticPool ?? LogRecordSharedPool.Current;

diff --git a/src/OpenTelemetry/ProviderExtensions.cs b/src/OpenTelemetry/ProviderExtensions.cs
@@ -73,6 +73,10 @@ public static Resource GetDefaultResource(this BaseProvider baseProvider)
             {
                 return meterProviderSdk.ServiceProvider;
             }
+            else if (baseProvider is OpenTelemetryLoggerProvider openTelemetryLoggerProvider)
+            {
+                return openTelemetryLoggerProvider.ServiceProvider;
+            }
 
             return null;
         }

diff --git a/src/OpenTelemetry/Resources/ResourceBuilder.cs b/src/OpenTelemetry/Resources/ResourceBuilder.cs
@@ -132,18 +132,24 @@ public ResourceBuilder AddDetector(IResourceDetector resourceDetector)
         /// <summary>
         /// Add a <see cref="IResourceDetector"/> to the builder which will be resolved using the application <see cref="IServiceProvider"/>.
         /// </summary>
-        /// <remarks>
-        /// Note: The supplied <paramref name="resourceDetectorFactory"/> may be
-        /// called with a <see langword="null"/> <see cref="IServiceProvider"/>
-        /// for detached <see cref="ResourceBuilder"/> instances. Factories
-        /// should either throw if a <see langword="null"/> cannot be handled,
-        /// or return a default <see cref="IResourceDetector"/> when <see
-        /// cref="IServiceProvider"/> is not available.
-        /// </remarks>
         /// <param name="resourceDetectorFactory">Resource detector factory.</param>
         /// <returns>Supplied <see cref="ResourceBuilder"/> for call chaining.</returns>
-        // Note: This API may be made public if there is a need for it.
-        internal ResourceBuilder AddDetector(Func<IServiceProvider?, IResourceDetector> resourceDetectorFactory)
+        public ResourceBuilder AddDetector(Func<IServiceProvider, IResourceDetector> resourceDetectorFactory)
+        {
+            Guard.ThrowIfNull(resourceDetectorFactory);
+
+            return this.AddDetectorInternal(sp =>
+            {
+                if (sp == null)
+                {
+                    throw new NotSupportedException("IResourceDetector factory pattern is not supported when calling ResourceBuilder.Build() directly.");
+                }
+
+                return resourceDetectorFactory(sp);
+            });
+        }
+
+        internal ResourceBuilder AddDetectorInternal(Func<IServiceProvider?, IResourceDetector> resourceDetectorFactory)
         {
             Guard.ThrowIfNull(resourceDetectorFactory);
 

diff --git a/src/OpenTelemetry/Resources/ResourceBuilderExtensions.cs b/src/OpenTelemetry/Resources/ResourceBuilderExtensions.cs
@@ -122,8 +122,8 @@ public static ResourceBuilder AddEnvironmentVariableDetector(this ResourceBuilde
             Lazy<IConfiguration> configuration = new Lazy<IConfiguration>(() => new ConfigurationBuilder().AddEnvironmentVariables().Build());
 
             return resourceBuilder
-                .AddDetector(sp => new OtelEnvResourceDetector(sp?.GetService<IConfiguration>() ?? configuration.Value))
-                .AddDetector(sp => new OtelServiceNameEnvVarDetector(sp?.GetService<IConfiguration>() ?? configuration.Value));
+                .AddDetectorInternal(sp => new OtelEnvResourceDetector(sp?.GetService<IConfiguration>() ?? configuration.Value))
+                .AddDetectorInternal(sp => new OtelServiceNameEnvVarDetector(sp?.GetService<IConfiguration>() ?? configuration.Value));
         }
 
         private static string GetFileVersion()

diff --git a/test/OpenTelemetry.Tests/Metrics/MeterProviderBuilderExtensionsTests.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderBuilderExtensionsTests.cs
@@ -108,7 +108,7 @@ public void SetAndConfigureResourceTest()
 
                 Assert.Empty(builder.ResourceDetectors);
 
-                builder.AddDetector(sp =>
+                builder.AddDetectorInternal(sp =>
                 {
                     serviceProviderTestExecuted = true;
                     Assert.NotNull(sp);

diff --git a/test/OpenTelemetry.Tests/Resources/ResourceTest.cs b/test/OpenTelemetry.Tests/Resources/ResourceTest.cs
@@ -498,13 +498,39 @@ public void GetResource_WithServiceNameSetWithTwoEnvVarsAndCode()
         }
 
         [Fact]
-        public void ResourceBuilder_ServiceProvider_Available()
+        public void ResourceBuilder_AddDetector_Test()
+        {
+            bool factoryExecuted = false;
+
+            var builder = ResourceBuilder.CreateDefault();
+
+            builder.AddDetector(sp =>
+            {
+                factoryExecuted = true;
+                return new NoopResourceDetector();
+            });
+
+            Assert.Throws<NotSupportedException>(() => builder.Build());
+            Assert.False(factoryExecuted);
+
+            var serviceCollection = new ServiceCollection();
+            using var serviceProvider = serviceCollection.BuildServiceProvider();
+
+            builder.ServiceProvider = serviceProvider;
+
+            var resource = builder.Build();
+
+            Assert.True(factoryExecuted);
+        }
+
+        [Fact]
+        public void ResourceBuilder_AddDetectorInternal_Test()
         {
             var builder = ResourceBuilder.CreateDefault();
 
             bool nullTestRun = false;
 
-            builder.AddDetector(sp =>
+            builder.AddDetectorInternal(sp =>
             {
                 nullTestRun = true;
                 Assert.Null(sp);
@@ -524,7 +550,7 @@ public void ResourceBuilder_ServiceProvider_Available()
 
             builder.ServiceProvider = serviceProvider;
 
-            builder.AddDetector(sp =>
+            builder.AddDetectorInternal(sp =>
             {
                 validTestRun = true;
                 Assert.NotNull(sp);

diff --git a/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs b/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTest.cs
@@ -196,7 +196,7 @@ public void SetAndConfigureResourceTest()
 
                 Assert.Empty(builder.ResourceDetectors);
 
-                builder.AddDetector(sp =>
+                builder.AddDetectorInternal(sp =>
                 {
                     serviceProviderTestExecuted = true;
                     Assert.NotNull(sp);