Blueprint for the path to stabilizing the OTLP log exporter

[alanwest]

Related to #4388

@vishweshbankwar @CodeBlanch

I do not intend for this PR to merge. I'm just using it to communicate what I've been thinking about since our SIG conversation this week.

I'd like us to consider delaying the decision to create a new base class for OTLP options (#4391). I recognize that we have some patterns for configuring the OTLP exporter that we are not happy with, and in order to get things on a track we're more happy with it may require a new base class + signal-specific option classes.

My hope is that we can decouple "addressing patterns we don't like" from "let's ship a stable OTLP log exporter".

In this PR I introduce some new option classes that mimics the pattern we used for metrics (i.e., MetricReaderOptions separate from OtlpExporterOptions).

namespace OpenTelemetry.Logs
{
+   public class LogRecordExportProcessorOptions
    {
+       public ExportProcessorType ExportProcessorType { get; set; }
+       public BatchExportLogRecordProcessorOptions BatchExportProcessorOptions { get; set; }
    }

    // This class will support the environment variables for logs i.e., OTEL_BLRP_*
+   public class BatchExportLogRecordProcessorOptions : BatchExportProcessorOptions<LogRecord>
    {

    }
}

Furthermore, we would have corresponding classes for Activity, allowing us to obsolete the processor-specific options currently on OtlpExporterOptions.

[alanwest]

This highlights one reason we should not attempt to ship a stable OTLP exporter without first integrating LoggerProviderBuilder currently on the main-logs branch.

[alanwest]

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.

[vishweshbankwar]

@alanwest Just adding some questions to clarify my understanding

1. decouple "addressing patterns we don't like" from "let's ship a stable OTLP log exporter" --> I agree. which one should be done first?
2. LogRecordExportProcessorOptions will resolve BatchExportProcessorOptions for otlp log exporter #4388. We will still have a gap of supporting OTEL_EXPORTER_OTLP_SIGNAL_* environment variables, Is my understanding correct?
3. New extensions on Logs and Activity that takes in Action<OtlpExporterOptions> and Action<Log/ActivityBatchExportProcessorOptions> will also potentially be obsolete when we attempt to solve 2)?

[alanwest]

@alanwest Just adding some questions to clarify my understanding

1. decouple "addressing patterns we don't like" from "let's ship a stable OTLP log exporter" --> I agree. which one should be done first?

Shipping the OTLP exporter and required support from the API/SDK should be done first. With the logs spec now stable (just happened this morning), I think we should aim to release stable logs as soon as we can. To me, this means not addressing patterns we don't like until after the first initial stable release.

2. LogRecordExportProcessorOptions will resolve BatchExportProcessorOptions for otlp log exporter #4388. We will still have a gap of supporting OTEL_EXPORTER_OTLP_SIGNAL_* environment variables, Is my understanding correct?

Yes, this is correct. I do not think that fixing this gap should be a priority for the initial stable release. The solutions that have been proposed to address this gap are some what disruptive i.e., introducing a new base class, signal-specific option classes, adding new extension methods possibly utilizing a new builder pattern, and perhaps obsoleting old extension methods. I think it would be ideal to try out these ideas after the initial stable release. The reason is because I think it would be good to have this work sit in pre-release for a decent amount of time so we get as much feedback as possible.

3. New extensions on Logs and Activity that takes in Action<OtlpExporterOptions> and Action<Log/ActivityBatchExportProcessorOptions> will also potentially be obsolete when we attempt to solve 2)?

Yes, I think solving 2 may involve obsoleting some extension methods. Though, in the immediate term for the initial stable release I feel strongly that we should obsolete the Activity-specific options currently on `OtlpExporterOptions - they have already caused confusion with metrics and they will cause even more confusion for logs.

[vishweshbankwar]

@alanwest Just adding some questions to clarify my understanding

1. decouple "addressing patterns we don't like" from "let's ship a stable OTLP log exporter" --> I agree. which one should be done first?

Shipping the OTLP exporter and required support from the API/SDK should be done first. With the logs spec now stable (just happened this morning), I think we should aim to release stable logs as soon as we can. To me, this means not addressing patterns we don't like until after the first initial stable release.

2. LogRecordExportProcessorOptions will resolve BatchExportProcessorOptions for otlp log exporter #4388. We will still have a gap of supporting OTEL_EXPORTER_OTLP_SIGNAL_* environment variables, Is my understanding correct?

Yes, this is correct. I do not think that fixing this gap should be a priority for the initial stable release. The solutions that have been proposed to address this gap are some what disruptive i.e., introducing a new base class, signal-specific option classes, adding new extension methods possibly utilizing a new builder pattern, and perhaps obsoleting old extension methods. I think it would be ideal to try out these ideas after the initial stable release. The reason is because I think it would be good to have this work sit in pre-release for a decent amount of time so we get as much feedback as possible.

3. New extensions on Logs and Activity that takes in Action<OtlpExporterOptions> and Action<Log/ActivityBatchExportProcessorOptions> will also potentially be obsolete when we attempt to solve 2)?

Yes, I think solving 2 may involve obsoleting some extension methods. Though, in the immediate term for the initial stable release I feel strongly that we should obsolete the Activity-specific options currently on `OtlpExporterOptions - they have already caused confusion with metrics and they will cause even more confusion for logs.

Sounds good! @CodeBlanch Does this sound good to you? If yes, then I can create an issue to note down the clear plan.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day.