[Logs] Updates for specification changes
Fixes #3637
[Apologies for the code bomb! 💣 💥 🤯]
Changes
-
The OpenTelemetry specification now has API definitions for
LoggerProvider&Logger. This PR adds those to the API project and moves some of what was in SDK into API (LogRecordData+LogRecordAttributeList). What is in API is very faithful to the spec, I didn't bring in any of the ILogger-specific stuff SDK has been using.- The spec is still experimental so most everything is
internal. Only the class definitions arepublicat the moment. I tried to make everything internal but that became way more work than it was worth. Making the classes at least public lets all the standard extensions work.
- The spec is still experimental so most everything is
-
There is now a dedicated
LoggerProviderSdkwhich contains much of whatOpenTelemetryLoggerProvidercontained previously. OurILoggersupport is now more of an integration than a fundamental thing. The entry point to logging is theLoggerProviderand that is fed into theILoggerProviderintegration in the same way it is fed into the Serilog or the EventSource adapter things. -
All of the extension methods added in #3504 have been moved to
LoggerProviderBuilder. There really isn't a difference between logs, traces, & metrics now. All the options that were specific to the ILogger integration are now separate from the provider. -
LogEmitteris gone.LoggerAPI now has methods for emitting logs + events. Serilog & EventSource projects now use those. -
Refactored some of the builder state & service registration helpers so all signals can use them.
-
Tried to improve the SDK
LogRecordas far as its spec compliance.- Added comments for what is ILogger-specific.
- Obsoleted
StateValuesin favor ofAttributes. - Added
Bodywhich is now set to{OriginalFormat}automatically for ILogger messages if it is detected as the last element in attributes. - Obsoleted
Stateby makingAttributesmore useful (see next bullet). - No longer automatically populate
TraceStateonLogRecords. That wasn't in the spec. I added an option to turn it back on if desired.
-
State+StateValues+ParseStateValuesThese things evolved to be strange. Exporters are having to check
StateValues&Stateand some are forcingParseStateValues = true.Stateitself is not safe to access beyond the log lifecycle. I tweaked things so there is a more deterministic behavior. IfTStateisIReadOnlyList<KeyValuePair<string, object>>(orIEnumerable<>equivalent) thanLogRecord.Attributeswill now always be populated. That will cover most messages written throughILoggerusing the source generator or the extensions. The only way to pass something that doesn't meet that requirement is callingILogger.Log<T>(...)directly. In that case ifParseStateValues = truethan we will buildAttributesdynamically using reflection.This allows for the deprecation of
LogRecord.State. Exporters should now be able to look atLogRecord.Attributesfor everything and get a nice and consistent behavior.If users don't care for export of attributes at all there is now an option
IncludeAttributesto turn off all operations againstTState.
More coming soon.
Public API Changes
Coming soon.
TODOs
- [ ] Appropriate
CHANGELOG.mdupdated for non-trivial changes - [ ] Design discussion issue #
- [ ] Changes in public API reviewed
Codecov Report
Merging #3677 (a64ef1b) into main (988a27b) will decrease coverage by
0.32%. The diff coverage is86.21%.
:exclamation: Current head a64ef1b differs from pull request most recent head a205903. Consider uploading reports for the commit a205903 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## main #3677 +/- ##
==========================================
- Coverage 87.74% 87.41% -0.33%
==========================================
Files 283 295 +12
Lines 10286 10542 +256
==========================================
+ Hits 9025 9215 +190
- Misses 1261 1327 +66
| Impacted Files | Coverage Δ | |
|---|---|---|
| ...Telemetry.Api/Internal/Shims/NullableAttributes.cs | 0.00% <ø> (ø) |
|
| src/OpenTelemetry.Api/Logs/NoopLogger.cs | 0.00% <0.00%> (ø) |
|
| ...nTelemetry/Internal/OpenTelemetrySdkEventSource.cs | 77.95% <25.00%> (-1.89%) |
:arrow_down: |
| ...ryProtocol.Logs/OtlpLogExporterHelperExtensions.cs | 61.11% <33.33%> (-30.56%) |
:arrow_down: |
| src/OpenTelemetry/Logs/LoggerSdk.cs | 40.00% <40.00%> (ø) |
|
| src/OpenTelemetry/Logs/LogRecord.cs | 60.50% <50.00%> (-4.85%) |
:arrow_down: |
| src/OpenTelemetry.Api/Logs/LoggerOptions.cs | 72.72% <72.72%> (ø) |
|
| ...c/OpenTelemetry.Api/Logs/LogRecordAttributeList.cs | 77.39% <77.39%> (ø) |
|
| src/OpenTelemetry.Api/InstrumentationScope.cs | 81.81% <81.81%> (ø) |
|
| src/OpenTelemetry.Api/Logs/LoggerProvider.cs | 83.33% <83.33%> (ø) |
|
| ... and 35 more |
Going to reopen this against a dedicated branch.