opentelemetry-dotnet
opentelemetry-dotnet copied to clipboard
Move LoggerProvider and friends (OTEL1000) to a stable API
Fixes #5442 Fixes #4014
Changes
- Expose the experimental APIs covered by OTEL1000 diagnostic
- Obsolete old APIs
Migration guidance for users
These two patterns will now generate warnings due to obsolete APIs:
appBuilder.Logging.AddOpenTelemetry(options =>
{
options.IncludeFormattedMessage = true;
options.AddOtlpExporter(o => o.Endpoint = new Uri("http://myotlpendpoint:4317/logs"));
});
using var loggerFactory = LoggerFactory.Create(builder => builder
.AddOpenTelemetry(options =>
{
options.IncludeFormattedMessage = true;
options.AddOtlpExporter(o => o.Endpoint = new Uri("http://myotlpendpoint:4317/logs"));
});
Migrate these to call UseOpenTelemetry instead:
appBuilder.Logging.UseOpenTelemetry(
logging => logging.AddOtlpExporter(o => o.Endpoint = new Uri("http://myotlpendpoint:4317/logs")),
options => options.IncludeFormattedMessage = true);
using var loggerFactory = LoggerFactory.Create(builder => builder
.UseOpenTelemetry(
logging => logging.AddOtlpExporter(o => o.Endpoint = new Uri("http://myotlpendpoint:4317/logs")),
options => options.IncludeFormattedMessage = true));
For this to work, your exporter needs to expose an extension which targets LoggerProviderBuilder. Extensions are provided for OtlpExporter, ConsoleExporter, and InMemoryExporter.
If you are using an exporter from the https://github.com/open-telemetry/opentelemetry-dotnet-contrib repository or some other exporter, it may not provide this extension yet. First try to upgrade your exporter package. If there isn't an update available, you may use this pattern to resolve obsoletion warnings by continuing to call the existing extension (while waiting for the component to be updated):
appBuilder.Logging.UseOpenTelemetry(
logging => { },
options =>
{
options.IncludeFormattedMessage = true;
// Note: Existing extensions targeting OpenTelemetryLoggerOptions can still be called.
options.AddGenevaLogExporter(o => o.ConnectionString = "...");
});
Migration guidance for library authors
Library authors should add extensions targeting LoggerProviderBuilder.
Before:
public static OpenTelemetryLoggerOptions AddMyExporter(
this OpenTelemetryLoggerOptions loggerOptions,
Action<MyExporterOptions> configure) {}
After:
[Obsolete("Call LoggerProviderBuilder.AddMyExporterinstead this method will be removed in a future version.")]
public static OpenTelemetryLoggerOptions AddMyExporter(
this OpenTelemetryLoggerOptions loggerOptions,
Action<MyExporterOptions> configure) {}
public static LoggerProviderBuilder AddMyExporter(
this LoggerProviderBuilder builder,
Action<MyExporterOptions> configure) {}
LoggerProviderBuilder has the same AddProcessor and SetResourceBuilder methods previously available on OpenTelemetryLoggerOptions as well as the full set of methods\features available to MeterProviderBuilder and TracerProviderBuilder.
Merge requirement checklist
- [X] CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
- [X] Unit tests added/updated
- [ ] Appropriate
CHANGELOG.mdfiles updated for non-trivial changes - [ ] Changes in public API reviewed (if applicable)
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 86.15%. Comparing base (
6250307) to head (6611356). Report is 249 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #5648 +/- ##
==========================================
+ Coverage 83.38% 86.15% +2.77%
==========================================
Files 297 254 -43
Lines 12531 11054 -1477
==========================================
- Hits 10449 9524 -925
+ Misses 2082 1530 -552
| Flag | Coverage Δ | |
|---|---|---|
| unittests | ? |
|
| unittests-Project-Experimental | 86.05% <100.00%> (?) |
|
| unittests-Project-Stable | 86.00% <100.00%> (?) |
|
| unittests-Solution | 86.01% <100.00%> (?) |
|
| unittests-UnstableCoreLibraries-Experimental | 20.19% <0.00%> (?) |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Files | Coverage Δ | |
|---|---|---|
| ...endencyInjectionLoggerProviderBuilderExtensions.cs | 100.00% <ø> (ø) |
|
| ...encyInjectionLoggingServiceCollectionExtensions.cs | 100.00% <ø> (ø) |
|
| src/OpenTelemetry.Api/Logs/LoggerProvider.cs | 100.00% <ø> (ø) |
|
| ...rc/OpenTelemetry.Api/Logs/LoggerProviderBuilder.cs | 100.00% <ø> (ø) |
|
| ...porter.Console/ConsoleExporterLoggingExtensions.cs | 0.00% <ø> (ø) |
|
| ...rter.InMemory/InMemoryExporterLoggingExtensions.cs | 100.00% <ø> (+33.33%) |
:arrow_up: |
| ...lemetryProtocol/OtlpLogExporterHelperExtensions.cs | 94.26% <ø> (+23.19%) |
:arrow_up: |
| ...lemetry.Extensions.Hosting/OpenTelemetryBuilder.cs | 100.00% <ø> (ø) |
|
| src/OpenTelemetry/BatchExportProcessor.cs | 84.11% <100.00%> (+1.86%) |
:arrow_up: |
| ...ry/Logs/Builder/LoggerProviderBuilderExtensions.cs | 100.00% <ø> (ø) |
|
| ... and 6 more |
Some thoughts for anyone taking a look at this...
-
We don't necessarily have to obsolete anything. I put them in initially to see the fallout. I added migration guidance to the PR so we can see what it would look like.
-
There is no
Sdk.CreateLoggerProviderBuilderbeing added here. I moved that to the Log Bridge experimental API (OTEL1001). The only use case I can think of forSdk.CreateLoggerProviderBuilderis in order to build aLoggerProvidermanually to feed into a bridge. For the use cases we have today,LoggerFactory.Createworks fine so I didn't feel this API was needed at this time.
Some thoughts for anyone taking a look at this...
- We don't necessarily have to obsolete anything. I put them in initially to see the fallout. I added migration guidance to the PR so we can see what it would look like.
- There is no
Sdk.CreateLoggerProviderBuilderbeing added here. I moved that to the Log Bridge experimental API (OTEL1001). The only use case I can think of forSdk.CreateLoggerProviderBuilderis in order to build aLoggerProvidermanually to feed into a bridge. For the use cases we have today,LoggerFactory.Createworks fine so I didn't feel this API was needed at this time.
@CodeBlanch - I think we should not mark things obsolete rn.
Regarding the migration guidance:
-
Migration guidance for case 1:
appBuilder.Logging.AddOpenTelemetry(options => { options.IncludeFormattedMessage = true; options.AddOtlpExporter(o => o.Endpoint = new Uri("http://myotlpendpoint:4317/logs")); });With this change, my understanding was, they should be able to move to following pattern (bringing consistency with other two signals for hosting scenarios and the update you have in the example)
appBuilder.Services.AddOpenTelemetry().WithLogging() -
Migration guidance for case 2:
using var loggerFactory = LoggerFactory.Create(builder => builder .UseOpenTelemetry( logging => logging.AddOtlpExporter(o => o.Endpoint = new Uri("http://myotlpendpoint:4317/logs")), options => options.IncludeFormattedMessage = true));I think we should not include it with this change. As per my understanding, it depends on the final outcome of https://github.com/open-telemetry/opentelemetry-dotnet/pull/5325. If we go ahead with the OpenTelemetrySdk pattern then we could just ask users to follow that (Separate discussion).
In short, the only use case we would enable right now is .WithLogging() for hosting scenarios.
For future, we could decide whether we want to have the migration guidance for case 2 or go with OpenTelemetrySdk pattern.
@vishweshbankwar
@CodeBlanch - I think we should not mark things obsolete rn.
Based on what thinking/reasons? Please elaborate a bit 😄
@vishweshbankwar
@CodeBlanch - I think we should not mark things obsolete rn.
Based on what thinking/reasons? Please elaborate a bit 😄
- Based on my point # 2: Once we have a clear plan/alternate for users on case 2 then we can consider the obsoleting part.
- Also, following this staged approach would give enough time for library authors also to support the extensions based on
LoggerProviderBuilder.
We decided on the SIG meeting 6/4/2024 that we would:
-
Not obsolete anything for
1.9.0. We will obsolete things in a future release. This was done to give library authors time to add extensions targetingLoggerProviderBuilderwhich will ease migration for users when things are obsoleted. -
ILoggingBuilder.UseOpenTelemetrywill not be exposed initially. It may be replaced by the design on #5325.
@alanwest @vishweshbankwar PR has been updated.
@CodeBlanch Can you update the PR description to include the changes expected to be done by end users as well, even though they are not forced to do it? Also call out that, this should be done only if their exporter supports this new model (and mention that exporters shipped from this repo already supports this model)
@cijothomas I added "Migration guidance for users" back on the PR description.