-
Notifications
You must be signed in to change notification settings - Fork 763
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
Moving IDeferredTracerProviderBuilder to API #2100
Moving IDeferredTracerProviderBuilder to API #2100
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2100 +/- ##
==========================================
+ Coverage 85.10% 85.12% +0.02%
==========================================
Files 187 187
Lines 6075 6078 +3
==========================================
+ Hits 5170 5174 +4
+ Misses 905 904 -1
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Merging quickly as this was discussed in the SIG meetings, and this is required for 1.0.0-beta5 release, (and to unblock 1.1.0 as well). |
Would you be open to having an interface, let's say EDIT: I'm referring to TracerProviderBuilderHosting that would eventually implement the interface. |
@danielcweber I don't think anything is off the table 😄 I'm not totally following you though. Could you post some pseudo-code (or whatever) to kind of demo how this would be used? One potential solution you can do right now... This extension allows you to register a callback action which will be fired when the opentelemetry-dotnet/src/OpenTelemetry.Extensions.Hosting/Trace/TracerProviderBuilderExtensions.cs Line 90 in 8c1c149
Now of course you need the public delegate void ConfigureTracerProviderAction(IServiceProvider serviceProvider, TracerProviderBuilder builder); Now we can use that extension above to (at "build" time) find extra external configurations and run them: services.AddOpenTelemetryTracing((builder) => builder
.Configure((sp, builder) =>
{
IEnumerable<ConfigureTracerProviderAction> registeredConfigurations = sp.GetServices<ConfigureTracerProviderAction>();
foreach (ConfigureTracerProviderAction registeredConfiguration in registeredConfigurations)
{
registeredConfiguration(sp, builder);
}
})); That allows you to then register anything you want to run with the services.AddTransient<MyInstrumentation>();
services.AddSingleton<ConfigureTracerProviderAction>((sp, builder) => builder
.AddInstrumentation(() => sp.GetRequiredService<MyInstrumentation>())); |
That's exactly the point. I don't want to have to configure something on Basically, I want to repeat the pattern seen here, but with an interface that would let me get the |
If you just want to make an extension method that configures + does service registrations, does this work? You have to take a dependency on OpenTelemetry.Extensions.Hosting to get access to all the fun extensions.
opentelemetry-dotnet/src/OpenTelemetry.Extensions.Hosting/Trace/TracerProviderBuilderExtensions.cs Lines 107 to 115 in 8c1c149
|
Awesome. I had no clue this extension existed. I should have searched for it more thoroughly. Thanks a lot. That should do! |
Related to #1889, #2058
Changes
Moved IDeferredTracerProviderBuilder from SDK to API. The goal is to allow instrumentation libraries to be written without a dependency on SDK.
Public API
API doesn't have a dependency on Microsoft.Extensions.DependencyInjection so IDeferredTracerProviderBuilder can no longer expose
IServiceCollection
.TODOs
CHANGELOG.md
updated for non-trivial changes