-
Notifications
You must be signed in to change notification settings - Fork 862
Blueprint for the path to stabilizing the OTLP log exporter #4416
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
Changes from all commits
5d2556c
c8ec489
b7f1547
e46c44e
1f15db8
181e59d
a9edbf4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| OpenTelemetry.Logs.OtlpLogExporterHelperExtensions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure, System.Action<OpenTelemetry.Logs.LogRecordExportProcessorOptions> configureProcessorOptions) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| OpenTelemetry.Logs.OtlpLogExporterHelperExtensions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure, System.Action<OpenTelemetry.Logs.LogRecordExportProcessorOptions> configureProcessorOptions) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| OpenTelemetry.Logs.OtlpLogExporterHelperExtensions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure, System.Action<OpenTelemetry.Logs.LogRecordExportProcessorOptions> configureProcessorOptions) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| OpenTelemetry.Logs.OtlpLogExporterHelperExtensions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions | ||
| static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions loggerOptions, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions> configure, System.Action<OpenTelemetry.Logs.LogRecordExportProcessorOptions> configureProcessorOptions) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,7 +14,6 @@ | |||||||
| // limitations under the License. | ||||||||
| // </copyright> | ||||||||
|
|
||||||||
| using System.Diagnostics; | ||||||||
| using OpenTelemetry.Exporter; | ||||||||
|
|
||||||||
| namespace OpenTelemetry.Logs | ||||||||
|
|
@@ -30,43 +29,61 @@ public static class OtlpLogExporterHelperExtensions | |||||||
| /// <remarks><inheritdoc cref="AddOtlpExporter(OpenTelemetryLoggerOptions, Action{OtlpExporterOptions})" path="/remarks"/></remarks> | ||||||||
| /// <param name="loggerOptions"><see cref="OpenTelemetryLoggerOptions"/> options to use.</param> | ||||||||
| /// <returns>The instance of <see cref="OpenTelemetryLoggerOptions"/> to chain the calls.</returns> | ||||||||
| [Obsolete("We will never ship this method as it is. We will ship the one that returns LoggerProviderBuilder from main-logs branch instead of OpenTelemetryLoggerOptions.")] | ||||||||
| public static OpenTelemetryLoggerOptions AddOtlpExporter(this OpenTelemetryLoggerOptions loggerOptions) | ||||||||
| => AddOtlpExporterInternal(loggerOptions, configure: null); | ||||||||
| => AddOtlpExporter(loggerOptions, configure: null, configureProcessorOptions: null); | ||||||||
|
|
||||||||
| /// <summary> | ||||||||
| /// Adds OTLP Exporter as a configuration to the OpenTelemetry ILoggingBuilder. | ||||||||
| /// </summary> | ||||||||
| /// <param name="loggerOptions"><see cref="OpenTelemetryLoggerOptions"/> options to use.</param> | ||||||||
| /// <param name="configure">Callback action for configuring <see cref="OtlpExporterOptions"/>.</param> | ||||||||
| /// <returns>The instance of <see cref="OpenTelemetryLoggerOptions"/> to chain the calls.</returns> | ||||||||
| [Obsolete("We will never ship this method as it is. We will ship one that returns LoggerProviderBuilder from main-logs branch instead of OpenTelemetryLoggerOptions.")] | ||||||||
| public static OpenTelemetryLoggerOptions AddOtlpExporter( | ||||||||
| this OpenTelemetryLoggerOptions loggerOptions, | ||||||||
| Action<OtlpExporterOptions> configure) | ||||||||
| => AddOtlpExporterInternal(loggerOptions, configure); | ||||||||
| => AddOtlpExporterInternal(loggerOptions, configure, configureProcessorOptions: null); | ||||||||
|
|
||||||||
| /// <summary> | ||||||||
| /// Adds OTLP Exporter as a configuration to the OpenTelemetry ILoggingBuilder. | ||||||||
| /// </summary> | ||||||||
| /// <param name="loggerOptions"><see cref="OpenTelemetryLoggerOptions"/> options to use.</param> | ||||||||
| /// <param name="configure">Callback action for configuring <see cref="OtlpExporterOptions"/>.</param> | ||||||||
| /// <param name="configureProcessorOptions">Callback action for configuring <see cref="LogRecordExportProcessorOptions"/>.</param> | ||||||||
| /// <returns>The instance of <see cref="OpenTelemetryLoggerOptions"/> to chain the calls.</returns> | ||||||||
| [Obsolete("We will never ship this method as it is. We will ship one that returns LoggerProviderBuilder from main-logs branch instead of OpenTelemetryLoggerOptions.")] | ||||||||
| public static OpenTelemetryLoggerOptions AddOtlpExporter( | ||||||||
| this OpenTelemetryLoggerOptions loggerOptions, | ||||||||
| Action<OtlpExporterOptions> configure, | ||||||||
| Action<LogRecordExportProcessorOptions> configureProcessorOptions) | ||||||||
|
Comment on lines
+56
to
+59
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This differs from what we did with metrics. Lines 112 to 114 in 10eb79c
That is, I've separated the delegates to configure the exporter and processor options. I'm a bit torn here, because I know we're unhappy with the pattern we used for metrics because it cannot be well supported by IOptions. But on the other hand, I don't like that we're introducing an inconsistency. One option may be to obsolete the extensions we have for metrics and add overloads that have a separate delegate for configuring |
||||||||
| => AddOtlpExporterInternal(loggerOptions, configure, configureProcessorOptions); | ||||||||
|
|
||||||||
| private static OpenTelemetryLoggerOptions AddOtlpExporterInternal( | ||||||||
| OpenTelemetryLoggerOptions loggerOptions, | ||||||||
| Action<OtlpExporterOptions> configure) | ||||||||
| Action<OtlpExporterOptions> configure, | ||||||||
| Action<LogRecordExportProcessorOptions> configureProcessorOptions) | ||||||||
| { | ||||||||
| var exporterOptions = new OtlpExporterOptions(); | ||||||||
| // TODO: This has not yet been adapted to correctly utilize IConfiguration/IOptions | ||||||||
| // It simply illustrates that the processor options will not come from | ||||||||
| // the obsolete properties on OtlpExporterOptions. | ||||||||
|
|
||||||||
| // TODO: We are using span/activity batch environment variable keys | ||||||||
| // here when we should be using the ones for logs. | ||||||||
| var defaultBatchOptions = exporterOptions.BatchExportProcessorOptions; | ||||||||
| var exporterOptions = new OtlpExporterOptions(); | ||||||||
|
|
||||||||
| Debug.Assert(defaultBatchOptions != null, "defaultBatchOptions was null"); | ||||||||
| var processorOptions = new LogRecordExportProcessorOptions(); | ||||||||
|
|
||||||||
| configure?.Invoke(exporterOptions); | ||||||||
| configureProcessorOptions?.Invoke(processorOptions); | ||||||||
|
|
||||||||
| var otlpExporter = new OtlpLogExporter(exporterOptions); | ||||||||
|
|
||||||||
| if (exporterOptions.ExportProcessorType == ExportProcessorType.Simple) | ||||||||
| if (processorOptions.ExportProcessorType == ExportProcessorType.Simple) | ||||||||
| { | ||||||||
| loggerOptions.AddProcessor(new SimpleLogRecordExportProcessor(otlpExporter)); | ||||||||
| } | ||||||||
| else | ||||||||
| { | ||||||||
| var batchOptions = exporterOptions.BatchExportProcessorOptions ?? defaultBatchOptions; | ||||||||
| var batchOptions = processorOptions.BatchExportProcessorOptions ?? new BatchExportLogRecordProcessorOptions(); | ||||||||
|
|
||||||||
| loggerOptions.AddProcessor(new BatchLogRecordExportProcessor( | ||||||||
| otlpExporter, | ||||||||
|
|
||||||||
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.
This highlights one reason we should not attempt to ship a stable OTLP exporter without first integrating
LoggerProviderBuildercurrently on themain-logsbranch.