Skip to content

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.

Sign up for GitHub

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

Closed
alanwest wants to merge 7 commits into from
Closed

Blueprint for the path to stabilizing the OTLP log exporter #4416

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5d2556c
Delete separate OTLP logs project
alanwest Apr 20, 2023
c8ec489
Add BatchExportLogRecordProcessorOptions
alanwest Apr 20, 2023
b7f1547
Add LogRecordExportProcessorOptions
alanwest Apr 20, 2023
e46c44e
Add ActivityExportProcessorOptions
alanwest Apr 20, 2023
1f15db8
Make AddOtlpExporter use LogRecordExportProcessorOptions
alanwest Apr 21, 2023
181e59d
Fix XML doc comment
alanwest Apr 21, 2023
a9edbf4
Obsolete OtlpExporterOptions.ExportProcessorType and BatchExportProce…
alanwest Apr 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Make AddOtlpExporter use LogRecordExportProcessorOptions
  • Loading branch information
@alanwest
alanwest committed Apr 21, 2023
commit 1f15db88b17000b8ab978d4fc0acd1076fd7532f
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/net462/PublicAPI.Unshipped.txt
View file
Open in desktop
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
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/net6.0/PublicAPI.Unshipped.txt
View file
Open in desktop
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
4 changes: 4 additions & 0 deletions ...elemetry.Exporter.OpenTelemetryProtocol/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt
View file
Open in desktop
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
4 changes: 4 additions & 0 deletions ...elemetry.Exporter.OpenTelemetryProtocol/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt
View file
Open in desktop
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
38 changes: 27 additions & 11 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
// limitations under the License.
// </copyright>

using System.Diagnostics;
using OpenTelemetry.Exporter;

namespace OpenTelemetry.Logs
Expand All @@ -30,43 +29,60 @@ 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)
Comment on lines +32 to 33
Copy link
Member Author

@alanwest alanwest Apr 21, 2023

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 LoggerProviderBuilder currently on the main-logs branch.

=> 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>
/// <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
Copy link
Member Author

@alanwest alanwest Apr 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This differs from what we did with metrics.

opentelemetry-dotnet/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpMetricExporterExtensions.cs

Lines 112 to 114 in 10eb79c

public static MeterProviderBuilder AddOtlpExporter(
this MeterProviderBuilder builder,
Action<OtlpExporterOptions, MetricReaderOptions> configureExporterAndMetricReader)

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 MetricReaderOptions.

=> 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,
Expand Down