Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DI] Remove ConfigureBuilder and add implementation factory helpers #4103

Merged

Conversation

CodeBlanch
Copy link
Member

@CodeBlanch CodeBlanch commented Jan 26, 2023

Changes

  • Adds a few standard helper extensions which were missing that would force users to call ConfigureBuilder.
  • Removes ConfigureBuilder.

Note: ConfigureBuilder functionality is still available by casting the builder to either IDeferredTracerProviderBuilder or IDeferredMeterProviderBuilder.

Public API Changes

OpenTelemetry.Extensions.DependencyInjection

namespace OpenTelemetry.Metrics
{
   public static class OpenTelemetryDependencyInjectionMeterProviderBuilderExtensions
   {
-       public static MeterProviderBuilder ConfigureBuilder(this MeterProviderBuilder meterProviderBuilder, Action<IServiceProvider, MeterProviderBuilder> configure) {}
+       internal static MeterProviderBuilder ConfigureBuilder(this MeterProviderBuilder meterProviderBuilder, Action<IServiceProvider, MeterProviderBuilder> configure) {}

        // Added earlier in 1.4.0
        public static MeterProviderBuilder AddInstrumentation<T>(this MeterProviderBuilder meterProviderBuilder, Func<IServiceProvider, T> instrumentationFactory) where T : class {}
   }
}

namespace OpenTelemetry.Trace
{
   public static class OpenTelemetryDependencyInjectionTracerProviderBuilderExtensions
   {
-       public static TracerProviderBuilder ConfigureBuilder(this TracerProviderBuilder tracerProviderBuilder, Action<IServiceProvider, TracerProviderBuilder> configure) {}
+       internal static TracerProviderBuilder ConfigureBuilder(this TracerProviderBuilder tracerProviderBuilder, Action<IServiceProvider, TracerProviderBuilder> configure) {}

        // Added earlier in 1.4.0
        public static TracerProviderBuilder AddInstrumentation<T>(this TracerProviderBuilder tracerProviderBuilder, Func<IServiceProvider, T> instrumentationFactory) where T : class {}
   }

OpenTelemetry

namespace OpenTelemetry.Metrics
{
   public static class MeterProviderBuilderExtensions
   {
+        public static MeterProviderBuilder AddReader(this MeterProviderBuilder meterProviderBuilder, Func<IServiceProvider, MetricReader> implementationFactory) {}
   }
}

namespace OpenTelemetry.Trace
{
   public static class TracerProviderBuilderExtensions
   {
+        public static TracerProviderBuilder SetSampler(this TracerProviderBuilder tracerProviderBuilder, Func<IServiceProvider, Sampler> implementationFactory) {}
+        public static TracerProviderBuilder AddProcessor(this TracerProviderBuilder tracerProviderBuilder, Func<IServiceProvider, BaseProcessor<Activity>> implementationFactory) {}
   }
}

TODOs

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Unit tests
  • Changes in public API reviewed

Note: Documentation updates will be done in a follow-up PR.

@codecov
Copy link

codecov bot commented Jan 26, 2023

Codecov Report

Merging #4103 (97f2964) into main (2e9a204) will increase coverage by 0.10%.
The diff coverage is 88.11%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    open-telemetry/opentelemetry-dotnet#4103      +/-   ##
==========================================
+ Coverage   85.52%   85.63%   +0.10%     
==========================================
  Files         293      293              
  Lines       11329    11359      +30     
==========================================
+ Hits         9689     9727      +38     
+ Misses       1640     1632       -8     
Impacted Files Coverage Δ
...xporter.Console/ConsoleExporterHelperExtensions.cs 0.00% <0.00%> (ø)
...porter.Console/ConsoleExporterMetricsExtensions.cs 0.00% <0.00%> (ø)
...pendencyInjectionMeterProviderBuilderExtensions.cs 100.00% <ø> (ø)
...endencyInjectionTracerProviderBuilderExtensions.cs 100.00% <ø> (ø)
....Hosting/Metrics/MeterProviderBuilderExtensions.cs 0.00% <0.00%> (ø)
...s.Hosting/Trace/TracerProviderBuilderExtensions.cs 0.00% <0.00%> (ø)
...emetry/Metrics/Builder/MeterProviderBuilderBase.cs 94.64% <ø> (+1.66%) ⬆️
...lemetry/Trace/Builder/TracerProviderBuilderBase.cs 78.66% <ø> (+1.03%) ⬆️
....Exporter.Jaeger/JaegerExporterHelperExtensions.cs 98.21% <80.00%> (ø)
...metryProtocol/OtlpTraceExporterHelperExtensions.cs 96.07% <80.00%> (ø)
... and 18 more

@CodeBlanch CodeBlanch changed the title [DI] Hide ConfigureBuilder and add implementation factory helpers [DI] Remove ConfigureBuilder and add implementation factory helpers Jan 28, 2023
@CodeBlanch CodeBlanch marked this pull request as ready for review January 28, 2023 18:40
@CodeBlanch CodeBlanch requested a review from a team January 28, 2023 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants