-
Notifications
You must be signed in to change notification settings - Fork 893
Dependency injection support #1889
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
Changes from 10 commits
ad49139
ee986e2
186fa2f
b99da7d
50212ac
960545e
1f75aac
670aa36
d5e2482
c5cf8da
f114261
beaeb93
3ff8258
3f264b6
415f133
9f28e45
3892b58
bba6131
0ce3374
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,22 +38,35 @@ public static TracerProviderBuilder AddZipkinExporter(this TracerProviderBuilder | |
| throw new ArgumentNullException(nameof(builder)); | ||
| } | ||
|
|
||
| var exporterOptions = new ZipkinExporterOptions(); | ||
| configure?.Invoke(exporterOptions); | ||
| var zipkinExporter = new ZipkinExporter(exporterOptions); | ||
| if (builder is IDeferredTracerBuilder deferredTracerBuilder) | ||
| { | ||
| return deferredTracerBuilder.Configure((sp, builder) => | ||
| { | ||
| AddZipkinExporter(builder, sp.GetOptions<ZipkinExporterOptions>(), configure); | ||
| }); | ||
| } | ||
|
|
||
| return AddZipkinExporter(builder, new ZipkinExporterOptions(), configure); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I personally don't like how this approach is causing all exporters and instrumentations to have to type check the builder and then have different code paths. IMO, it shouldn't be optional for these exporters and instrumentations to support separating configuration and construction phases. It should be mandatory that they register things with a deferred approach so that we can keep configuration and construction phases cleanly separated.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would personally prefer if we could force all of them to be registered with lambdas and not even make it an option to immediately new them up when configuring. Using a factory method approach for configuration is compatible with all targeted platforms without taking a dependency on
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of everything on this PR, I'm the least happy with this bit. But I don't think it is possible to do it any other way 😭 It's not really a problem of separating configuration and construction, this is about the inversion of dependencies we've created for ourselves. To do it "properly" we would need this ctor: |
||
| } | ||
|
|
||
| private static TracerProviderBuilder AddZipkinExporter(TracerProviderBuilder builder, ZipkinExporterOptions options, Action<ZipkinExporterOptions> configure = null) | ||
| { | ||
| configure?.Invoke(options); | ||
|
|
||
| var zipkinExporter = new ZipkinExporter(options); | ||
|
|
||
| if (exporterOptions.ExportProcessorType == ExportProcessorType.Simple) | ||
| if (options.ExportProcessorType == ExportProcessorType.Simple) | ||
| { | ||
| return builder.AddProcessor(new SimpleActivityExportProcessor(zipkinExporter)); | ||
| } | ||
| else | ||
| { | ||
| return builder.AddProcessor(new BatchActivityExportProcessor( | ||
| zipkinExporter, | ||
| exporterOptions.BatchExportProcessorOptions.MaxQueueSize, | ||
| exporterOptions.BatchExportProcessorOptions.ScheduledDelayMilliseconds, | ||
| exporterOptions.BatchExportProcessorOptions.ExporterTimeoutMilliseconds, | ||
| exporterOptions.BatchExportProcessorOptions.MaxExportBatchSize)); | ||
| options.BatchExportProcessorOptions.MaxQueueSize, | ||
| options.BatchExportProcessorOptions.ScheduledDelayMilliseconds, | ||
| options.BatchExportProcessorOptions.ExporterTimeoutMilliseconds, | ||
| options.BatchExportProcessorOptions.MaxExportBatchSize)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,11 @@ | ||
| Microsoft.Extensions.DependencyInjection.OpenTelemetryServicesExtensions | ||
| OpenTelemetry.Trace.TracerProviderBuilderExtensions | ||
| OpenTelemetry.Trace.TracerProviderBuilderHosting | ||
| OpenTelemetry.Trace.TracerProviderBuilderHosting.Services.get -> Microsoft.Extensions.DependencyInjection.IServiceCollection | ||
| static Microsoft.Extensions.DependencyInjection.OpenTelemetryServicesExtensions.AddOpenTelemetryTracing(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection | ||
| static Microsoft.Extensions.DependencyInjection.OpenTelemetryServicesExtensions.AddOpenTelemetryTracing(this Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Action<OpenTelemetry.Trace.TracerProviderBuilder> configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection | ||
| static Microsoft.Extensions.DependencyInjection.OpenTelemetryServicesExtensions.AddOpenTelemetryTracing(this Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Action<System.IServiceProvider, OpenTelemetry.Trace.TracerProviderBuilder> configure) -> Microsoft.Extensions.DependencyInjection.IServiceCollection | ||
| static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddInstrumentation<T>(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder) -> OpenTelemetry.Trace.TracerProviderBuilder | ||
| static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddProcessor<T>(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder) -> OpenTelemetry.Trace.TracerProviderBuilder | ||
| static OpenTelemetry.Trace.TracerProviderBuilderExtensions.Build(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, System.IServiceProvider serviceProvider) -> OpenTelemetry.Trace.TracerProvider | ||
| static OpenTelemetry.Trace.TracerProviderBuilderExtensions.Configure(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, System.Action<System.IServiceProvider, OpenTelemetry.Trace.TracerProviderBuilder> configure) -> OpenTelemetry.Trace.TracerProviderBuilder | ||
| static OpenTelemetry.Trace.TracerProviderBuilderExtensions.SetSampler<T>(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder) -> OpenTelemetry.Trace.TracerProviderBuilder |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| // <copyright file="TracerProviderBuilderExtensions.cs" company="OpenTelemetry Authors"> | ||
| // Copyright The OpenTelemetry Authors | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
| // </copyright> | ||
|
|
||
| using System; | ||
| using System.Diagnostics; | ||
|
|
||
| namespace OpenTelemetry.Trace | ||
| { | ||
| /// <summary> | ||
| /// Contains extension methods for the <see cref="TracerProviderBuilder"/> class. | ||
| /// </summary> | ||
| public static class TracerProviderBuilderExtensions | ||
| { | ||
| /// <summary> | ||
| /// Adds instrumentation to the provider. | ||
| /// </summary> | ||
| /// <typeparam name="T">Instrumentation type.</typeparam> | ||
| /// <param name="tracerProviderBuilder"><see cref="TracerProviderBuilder"/>.</param> | ||
| /// <returns>The supplied <see cref="TracerProviderBuilder"/> for chaining.</returns> | ||
| public static TracerProviderBuilder AddInstrumentation<T>(this TracerProviderBuilder tracerProviderBuilder) | ||
| where T : class | ||
| { | ||
| if (tracerProviderBuilder is TracerProviderBuilderHosting tracerProviderBuilderHosting) | ||
| { | ||
| tracerProviderBuilderHosting.AddInstrumentation<T>(); | ||
| } | ||
|
|
||
| return tracerProviderBuilder; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Adds a processor to the provider. | ||
| /// </summary> | ||
| /// <typeparam name="T">Processor type.</typeparam> | ||
| /// <param name="tracerProviderBuilder"><see cref="TracerProviderBuilder"/>.</param> | ||
| /// <returns>The supplied <see cref="TracerProviderBuilder"/> for chaining.</returns> | ||
| public static TracerProviderBuilder AddProcessor<T>(this TracerProviderBuilder tracerProviderBuilder) | ||
| where T : BaseProcessor<Activity> | ||
| { | ||
| if (tracerProviderBuilder is TracerProviderBuilderHosting tracerProviderBuilderHosting) | ||
| { | ||
| tracerProviderBuilderHosting.AddProcessor<T>(); | ||
| } | ||
|
|
||
| return tracerProviderBuilder; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Sets the sampler on the provider. | ||
| /// </summary> | ||
| /// <typeparam name="T">Sampler type.</typeparam> | ||
| /// <param name="tracerProviderBuilder"><see cref="TracerProviderBuilder"/>.</param> | ||
| /// <returns>The supplied <see cref="TracerProviderBuilder"/> for chaining.</returns> | ||
| public static TracerProviderBuilder SetSampler<T>(this TracerProviderBuilder tracerProviderBuilder) | ||
| where T : Sampler | ||
| { | ||
| if (tracerProviderBuilder is TracerProviderBuilderHosting tracerProviderBuilderHosting) | ||
| { | ||
| tracerProviderBuilderHosting.SetSampler<T>(); | ||
| } | ||
|
|
||
| return tracerProviderBuilder; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Register a callback action to configure the <see cref="TracerProviderBuilder"/> during initialization. | ||
| /// </summary> | ||
| /// <param name="tracerProviderBuilder"><see cref="TracerProviderBuilder"/>.</param> | ||
| /// <param name="configure">Configuration callback.</param> | ||
| /// <returns>The supplied <see cref="TracerProviderBuilder"/> for chaining.</returns> | ||
| public static TracerProviderBuilder Configure(this TracerProviderBuilder tracerProviderBuilder, Action<IServiceProvider, TracerProviderBuilder> configure) | ||
| { | ||
| if (tracerProviderBuilder is IDeferredTracerBuilder deferredTracerBuilder) | ||
| { | ||
| deferredTracerBuilder.Configure(configure); | ||
| } | ||
|
|
||
| return tracerProviderBuilder; | ||
| } | ||
|
CodeBlanch marked this conversation as resolved.
|
||
|
|
||
| /// <summary> | ||
| /// Run the configured actions to initialize the <see cref="TracerProvider"/>. | ||
| /// </summary> | ||
| /// <param name="tracerProviderBuilder"><see cref="TracerProviderBuilder"/>.</param> | ||
| /// <param name="serviceProvider"><see cref="IServiceProvider"/>.</param> | ||
| /// <returns><see cref="TracerProvider"/>.</returns> | ||
| public static TracerProvider Build(this TracerProviderBuilder tracerProviderBuilder, IServiceProvider serviceProvider) | ||
| { | ||
| if (tracerProviderBuilder is TracerProviderBuilderHosting tracerProviderBuilderHosting) | ||
| { | ||
| return tracerProviderBuilderHosting.Build(serviceProvider); | ||
| } | ||
|
|
||
| if (tracerProviderBuilder is TracerProviderBuilderBase tracerProviderBuilderBase) | ||
| { | ||
| return tracerProviderBuilderBase.Build(); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.