opentelemetry-dotnet icon indicating copy to clipboard operation
opentelemetry-dotnet copied to clipboard

[Logs] Updates for specification changes

Open CodeBlanch opened this issue 3 years ago • 1 comments

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 are public at 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.
  • There is now a dedicated LoggerProviderSdk which contains much of what OpenTelemetryLoggerProvider contained previously. Our ILogger support is now more of an integration than a fundamental thing. The entry point to logging is the LoggerProvider and that is fed into the ILoggerProvider integration 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.

  • LogEmitter is gone. Logger API 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 LogRecord as far as its spec compliance.

    • Added comments for what is ILogger-specific.
    • Obsoleted StateValues in favor of Attributes.
    • Added Body which is now set to {OriginalFormat} automatically for ILogger messages if it is detected as the last element in attributes.
    • Obsoleted State by making Attributes more useful (see next bullet).
    • No longer automatically populate TraceState on LogRecords. That wasn't in the spec. I added an option to turn it back on if desired.
  • State + StateValues + ParseStateValues

    These things evolved to be strange. Exporters are having to check StateValues & State and some are forcing ParseStateValues = true. State itself is not safe to access beyond the log lifecycle. I tweaked things so there is a more deterministic behavior. If TState is IReadOnlyList<KeyValuePair<string, object>> (or IEnumerable<> equivalent) than LogRecord.Attributes will now always be populated. That will cover most messages written through ILogger using the source generator or the extensions. The only way to pass something that doesn't meet that requirement is calling ILogger.Log<T>(...) directly. In that case if ParseStateValues = true than we will build Attributes dynamically using reflection.

    This allows for the deprecation of LogRecord.State. Exporters should now be able to look at LogRecord.Attributes for everything and get a nice and consistent behavior.

    If users don't care for export of attributes at all there is now an option IncludeAttributes to turn off all operations against TState.

More coming soon.

Public API Changes

Coming soon.

TODOs

  • [ ] Appropriate CHANGELOG.md updated for non-trivial changes
  • [ ] Design discussion issue #
  • [ ] Changes in public API reviewed

CodeBlanch avatar Sep 19 '22 23:09 CodeBlanch

Codecov Report

Merging #3677 (a64ef1b) into main (988a27b) will decrease coverage by 0.32%. The diff coverage is 86.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

Impacted file tree graph

@@            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

codecov[bot] avatar Sep 19 '22 23:09 codecov[bot]

Going to reopen this against a dedicated branch.

CodeBlanch avatar Sep 29 '22 00:09 CodeBlanch