[Logs-branch] Add LoggerProvider spec changes #3707
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
reyang
approved these changes
Sep 29, 2022
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main-logs #3707 +/- ##
=============================================
+ Coverage 87.14% 87.27% +0.13%
=============================================
Files 274 296 +22
Lines 10008 10590 +582
=============================================
+ Hits 8721 9242 +521
- Misses 1287 1348 +61
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3637
Description
This PR brings the changes reverted on #3702 with additions from #3677 to the
main-logsbranch. Since it is now a dedicated branch things have been madepublicas if the spec was stable. Making things public is helpful to see the true shape and vet the usage 🔍Changes
LoggerProvider\LoggerAPI from the OpenTelemetry Specification.Body&AttributesonLogRecordand gives new meaning to theOpenTelemetryLoggerOptions.ParseStateValuesoption.Public API Changes
OpenTelemetry.Api
namespace OpenTelemetry.Logs { + public class LoggerProvider : BaseProvider + { + protected LoggerProvider() {} + public Logger GetLogger() {} + public Logger GetLogger(string? name) {} + public Logger GetLogger(InstrumentationScope instrumentationScope) {} + public virtual Logger GetLogger(LoggerOptions options) {} + } + public abstract class Logger + { + protected Logger(LoggerOptions options) {} + public abstract void EmitEvent(string name, in LogRecordData data, in LogRecordAttributeList attributes = default) {} + public abstract void EmitLog(in LogRecordData data, in LogRecordAttributeList attributes = default) {} + } + public struct LogRecordData + { + public LogRecordData() {} + public LogRecordData(Activity? activity) {} + public DateTime Timestamp { get; set; } + public ActivityTraceId TraceId { get; set; } + public ActivitySpanId SpanId { get; set; } + public ActivityTraceFlags TraceFlags { get; set; } + public LogRecordSeverity? Severity { get; set; } + public string? Body { get; set; } = null; + } + public enum LogRecordSeverity + { + Trace, + Debug, + Information, + Warning, + Error, + Fatal, + } + public struct LogRecordAttributeList : IReadOnlyList<KeyValuePair<string, object?>> + { + public int Count { get; } + public KeyValuePair<string, object?> this[int index] { get; set; } + [EditorBrowsable(EditorBrowsableState.Never)] public object? this[string key] { set; } + public void Add(string key, object? value) {} + public void Add(KeyValuePair<string, object?> attribute) {} + public void RecordException(Exception exception) {} + public Enumerator GetEnumerator() {} + IEnumerator<KeyValuePair<string, object?>> IEnumerable<KeyValuePair<string, object?>>.GetEnumerator() {} + IEnumerator IEnumerable.GetEnumerator() {} + public struct Enumerator : IEnumerator<KeyValuePair<string, object?>>, IEnumerator + { + public KeyValuePair<string, object?> Current { get; } + object IEnumerator.Current { get; } + public bool MoveNext() {} + public void Dispose() {} + void IEnumerator.Reset() {} + } + } + public sealed class LoggerOptions + { + public LoggerOptions() {} + public LoggerOptions(string? name) {} + public LoggerOptions(InstrumentationScope instrumentationScope) {} + public InstrumentationScope InstrumentationScope { get; } + public string? EventDomain { get; init; } + public bool IncludeTraceContext { get; init; } = true; + } + public abstract class LoggerProviderBuilder + { + protected LoggerProviderBuilder() {} + } } namespace OpenTelemetry { + public sealed class InstrumentationScope + { + public InstrumentationScope() {} + public InstrumentationScope(string? name) {} + public string Name { get; } + public string? Version { get; init; } + public string? SchemaUrl { get; init; } + public IReadOnlyDictionary<string, object>? Attributes { get; init; } + } }OpenTelemetry
namespace OpenTelemetry.Logs { public sealed class LogRecord { + public string? Body { get; set; } + public IReadOnlyList<KeyValuePair<string, object?>>? Attributes { get; set; } + public LogRecordSeverity? Severity { get; set; } + public InstrumentationScope? InstrumentationScope { get; } + [Obsolete("Use Severity instead LogLevel will be removed in a future version.")] public LogLevel LogLevel { get; set; } + [Obsolete("State cannot be accessed safely outside of an ILogger.Log call stack. It will be removed in a future version.")] public object? State { get; set; } + [Obsolete("Use Attributes instead StateValues will be removed in a future version.")] public IReadOnlyList<KeyValuePair<string, object?>>? StateValues { get; set; } } public class OpenTelemetryLoggerOptions { + public bool IncludeState { get; set; } = true; + public bool IncludeTraceState { get; set; } + [Obsolete("Use LoggerProviderBuilder instead of OpenTelemetryLoggerOptions to configure a LoggerProvider this method will be removed in a future version.")] public OpenTelemetryLoggerOptions AddProcessor(BaseProcessor<LogRecord> processor) {} + [Obsolete("Use LoggerProviderBuilder instead of OpenTelemetryLoggerOptions to configure a LoggerProvider this method will be removed in a future version.")] public OpenTelemetryLoggerOptions SetResourceBuilder(ResourceBuilder resourceBuilder) {} + [Obsolete("Use LoggerProviderBuilder instead of OpenTelemetryLoggerOptions to configure a LoggerProvider this method will be removed in a future version.")] public OpenTelemetryLoggerOptions ConfigureResource(Action<ResourceBuilder> configure) {} } + public static class LoggerProviderBuilderExtensions + { + public static LoggerProviderBuilder SetResourceBuilder(this LoggerProviderBuilder loggerProviderBuilder, ResourceBuilder resourceBuilder) {} + public static LoggerProviderBuilder ConfigureResource(this LoggerProviderBuilder loggerProviderBuilder, Action<ResourceBuilder> configure) {} + public static LoggerProviderBuilder AddProcessor(this LoggerProviderBuilder loggerProviderBuilder, BaseProcessor<LogRecord> processor) {} + public static LoggerProviderBuilder AddProcessor<T>(this LoggerProviderBuilder loggerProviderBuilder) where T : BaseProcessor<LogRecord> {} + public static LoggerProviderBuilder AddExporter(this LoggerProviderBuilder loggerProviderBuilder, ExportProcessorType exportProcessorType, BaseExporter<LogRecord> exporter) {} + public static LoggerProviderBuilder AddExporter(this LoggerProviderBuilder loggerProviderBuilder, ExportProcessorType exportProcessorType, BaseExporter<LogRecord> exporter, Action<ExportLogRecordProcessorOptions> configure) {} + public static LoggerProviderBuilder AddExporter(this LoggerProviderBuilder loggerProviderBuilder, ExportProcessorType exportProcessorType, BaseExporter<LogRecord> exporter, string? name, Action<ExportLogRecordProcessorOptions>? configure) {} + public static LoggerProviderBuilder AddExporter<T>(this LoggerProviderBuilder loggerProviderBuilder, ExportProcessorType exportProcessorType) where T : BaseExporter<LogRecord> {} + public static LoggerProviderBuilder AddExporter<T>(this LoggerProviderBuilder loggerProviderBuilder, ExportProcessorType exportProcessorType, Action<ExportLogRecordProcessorOptions> configure) where T : BaseExporter<LogRecord> {} + public static LoggerProviderBuilder AddExporter<T>(this LoggerProviderBuilder loggerProviderBuilder, ExportProcessorType exportProcessorType, string? name, Action<ExportLogRecordProcessorOptions>? configure) where T : BaseExporter<LogRecord> {} + public static LoggerProviderBuilder ConfigureServices(this LoggerProviderBuilder loggerProviderBuilder, Action<IServiceCollection> configure) {} + public static LoggerProviderBuilder ConfigureBuilder(this LoggerProviderBuilder loggerProviderBuilder, Action<IServiceProvider, LoggerProviderBuilder> configure) {} + public static LoggerProvider Build(this LoggerProviderBuilder loggerProviderBuilder) {} + public static LoggerProviderBuilder AddInstrumentation<T>(this LoggerProviderBuilder loggerProviderBuilder) where T : class {} + public static LoggerProviderBuilder AddInstrumentation<T>(this LoggerProviderBuilder loggerProviderBuilder, Func<IServiceProvider, LoggerProvider, T> instrumentationFactory) where T : class {} + } + public static class LoggerProviderExtensions + { + public static LoggerProvider AddProcessor(this LoggerProvider provider, BaseProcessor<LogRecord> processor) {} + public static bool ForceFlush(this LoggerProvider provider, int timeoutMilliseconds = Timeout.Infinite) {} + public static bool Shutdown(this LoggerProvider provider, int timeoutMilliseconds = Timeout.Infinite) {} + } + public static class LoggerProviderBuilderServiceCollectionExtensions + { + public static IServiceCollection ConfigureOpenTelemetryLogging(this IServiceCollection services) {} + public static IServiceCollection ConfigureOpenTelemetryLogging(this IServiceCollection services, Action<LoggerProviderBuilder>? configure) {} + } public static class OpenTelemetryLoggingExtensions { // The return type has change on these. See the section on backwards compatibility - public static ILoggingBuilder AddOpenTelemetry(this ILoggingBuilder builder) {} - public static ILoggingBuilder AddOpenTelemetry(this ILoggingBuilder builder, Action<OpenTelemetryLoggerOptions> configureOptions) {} + public static OpenTelemetryLoggingBuilder AddOpenTelemetry(this ILoggingBuilder builder) {} + public static OpenTelemetryLoggingBuilder AddOpenTelemetry(this ILoggingBuilder builder, Action<OpenTelemetryLoggerOptions> configureOptions) {} + public static ILoggingBuilder AddOpenTelemetry(this ILoggingBuilder builder, LoggerProvider loggerProvider) {} + public static ILoggingBuilder AddOpenTelemetry(this ILoggingBuilder builder, LoggerProvider loggerProvider, Action<OpenTelemetryLoggerOptions> configureOptions) {} + public static ILoggingBuilder AddOpenTelemetry(this ILoggingBuilder builder, LoggerProvider loggerProvider, Action<OpenTelemetryLoggerOptions>? configureOptions, bool disposeProvider) {} } // See the section on backwards compatibility + public sealed class OpenTelemetryLoggingBuilder : LoggerProviderBuilder, ILoggingBuilder + { + public IServiceCollection Services { get; } // Needed for ILoggingBuilder + } // This class is similar to MetricReaderOptions and is used by AddExporter extensions + public class ExportLogRecordProcessorOptions + { + public ExportProcessorType ExportProcessorType { get; set; } + public BatchExportLogRecordProcessorOptions BatchExportProcessorOptions { get; set; } + } // This class is similar to BatchExportActivityProcessorOptions and exists so separate environment variables may be used to configure batch log processor settings + public class BatchExportLogRecordProcessorOptions : BatchExportProcessorOptions<LogRecord> + { + } } namespace OpenTelemetry { public static class Sdk { + public static LoggerProviderBuilder CreateLoggerProviderBuilder() {} } }Also...
LoggerProviderBuilderAPI.Backwards compatibility
The stable API in 1.3.1 uses OpenTelemetryLoggerOptions as the builder for logging. There isn't much to it, only
AddProcessorandSetResourceBuilderare available. Extensions are coded to use that API when building up the provider. 1.4 is addingConfigureResource.This PR is essentially splitting things up so that
LoggingProviderBuilderis responsible for building the specLoggerProviderandOpenTelemetryLoggerOptionsis responsible for configuring theOpenTelemetryLoggerProviderwhich is theILoggerintegration.This was tricky and I spent a lot of time trying many different things. This is the best I could come up with! Open to suggestions 🤔
AddProcessor,SetResourceBuilder, &ConfigureResourcestill exist onOpenTelemetryLoggerOptions. Code and extensions that rely on them will continue to function. How this works is anything set on the options will be applied to the provider when it is ready.LoggingProviderBuilderhas all the great stuff now. This is the API we want to use instead. To encourage migrationAddProcessor,SetResourceBuilder, &ConfigureResourceare now marked asObsoleteas are the extensions usingOpenTelemetryLoggerOptions. New extensions have been added targetingLoggingProviderBuilderinstead.Here is some working 1.3.1/1.4 code:
That will continue to work but now generates warnings due to the obsoletions. Here is the new pattern:
AddOpenTelemetryreturnsOpenTelemetryLoggingBuilderwhich implementsLoggerProviderBuilderto configure the builder.This pattern works equally well:
As does...
The first pattern configures everything off of
ILoggingBuilderin one spot. The second two configureLoggerProviderindependently and then tellILoggerBuilder(ILogger integration) to use it. We don't have to provide a "one-spot" option, but if ILogger integration is our most common scenario it is kind of nice.TODOs
CHANGELOG.mdupdated for non-trivial changes