open-telemetry / opentelemetry-dotnet

The OpenTelemetry .NET Client
https://opentelemetry.io
Apache License 2.0
3.17k stars 751 forks source link

Builder pattern for OTLP Exporter configuration options #4479

Closed vishweshbankwar closed 1 year ago

vishweshbankwar commented 1 year ago

The current implementation of the OTLP exporter for traces, metrics, and logs (non-stable) has certain limitations and issues:

1) It does not support configuration via signal specific OTEL_EXPORTER_OTLPSIGNAL* environment variables and it is not possible to add support for it via common OtlpExporterOptions.

2) The APIs/Extension Methods are inconsistent across traces, metrics, and logs to configure the exporter pipeline.

To address these issues, proposing a builder pattern for configuring options in the OTLP exporter. This pattern would allow us to resolve the limitations and issues mentioned above and enable us to add more configuration options in the future if needed.

How the changes will affect OTLP exporter users

Traces Current State:

AddOtlpExporter(options => 
{
    options.Protocol = OtlpExportProtocol.Grpc;
    options.ExportProcessorType = ExportProcessorType.Simple;
})

Proposed State:

AddOtlpTraceExporter(builder =>
{
    builder.ConfigureExporterOptions(exporterOptions => exporterOptions.Protocol = OtlpExportProtocol.Grpc);
    builder.ConfigureProcessorOptions(processorOptions =>
    {
        processorOptions.ExportProcessorType = ExportProcessorType.Batch;
        processorOptions.BatchExportProcessorOptions = new BatchExportActivityProcessorOptions() { MaxQueueSize = 1000 };
    });
})

Metrics Current State:

AddOtlpExporter((options, metricReaderOptions) =>
{
    options.Protocol = OtlpExportProtocol.Grpc;
    metricReaderOptions.PeriodicExportingMetricReaderOptions = new PeriodicExportingMetricReaderOptions() { ExportIntervalMilliseconds = 30 };
})

Proposed State:

AddOtlpMetricExporter((builder) =>
{
    builder.ConfigureExporterOptions(exporterOptions => exporterOptions.Protocol = OtlpExportProtocol.Grpc);
    builder.ConfigureReaderOptions(readerOptions => readerOptions.PeriodicExportingMetricReaderOptions = new PeriodicExportingMetricReaderOptions() { ExportIntervalMilliseconds = 30 };
})

Logs (Non-stable) Current State:

AddOtlpExporter(options => 
{
    options.Protocol = OtlpExportProtocol.Grpc;
    opt.ExportProcessorType = ExportProcessorType.Simple;
})

Proposed State:

AddOtlpLogExporter(builder =>
{
    builder.ConfigureExporterOptions(exporterOptions => exporterOptions.Protocol = OtlpExportProtocol.Grpc);
    builder.ConfigureProcessorOptions(processorOptions =>
    {
        processorOptions.ExportProcessorType = ExportProcessorType.Batch;
        processorOptions.BatchExportProcessorOptions = new BatchExportLogRecordProcessorOptions() { MaxQueueSize = 1000 };
    });
})

Reference PR that shows the proposed changes for Traces

https://github.com/open-telemetry/opentelemetry-dotnet/pull/4470

alanwest commented 1 year ago

Here's a different option to discuss in today's SIG.

This flow seems more intuitive with regards to the spec and constructing the pipeline. It further decouples ExportProcesssor (spans and logs) and MetricReader (metrics) from being a concern that each exporter has to handle configuration for. I believe we're the only SDK that has mushed together processor/reader configuration into our programatic configuration API.

Metrics

var meterProviderBuilder = Sdk.CreateMeterProviderBuilder()
    .AddReader(metricReaderBuilder =>
    {
        metricReaderBuilder
            .ConfigureMetricReaderOptions(options =>
            {
                options.TemporalityPreference = MetricReaderTemporalityPreference.Delta;
                options.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds = 10000;
            })

            // The OTLP exporter would provide extension methods on MetricReaderBuilder rather than MeterProviderBuilder
            .AddOtlpExporter(options => // MetricOtlpExporterOptions
            {
                options.Endpoint = new Uri("http://my-collector.com");
            });
    })

Traces

var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder()
    .AddBatchExportProcessor(exportProcessorBuilder => // ActivityExportProcessorBuilder
    {
        exportProcessorBuilder
            .ConfigureBatchExportProcessorOptions(options =>
            {
                options.ExportIntervalMilliseconds = 10000;
            })

            // The OTLP exporter would provide extension methods on ExportProcessorBuilder rather than TracerProviderBuilder
            .AddOtlpExporter(options => // TraceOtlpExporterOptions
            {
                options.Endpoint = new Uri("http://my-collector.com");
            });
    });

Comparison to Java

Traces

var tracerProviderBuilder = SdkTracerProvider.builder();

var spanExporter = OtlpGrpcSpanExporter.builder()
    .setEndpoint("http://my-collector.com")
    .build();

var batchSpanProcessor = BatchSpanProcessor.builder(spanExporter).build();

var tracerProvider = tracerProviderBuilder.addSpanProcessor(batchSpanProcessor).build();

Metrics

var meterProviderBuilder = SdkMeterProvider.builder();

var metricExporter = OtlpGrpcMetricExporter.builder()
    .setEndpoint("http://my-collector.com")
    .build();

var metricReader = PeriodicMetricReader.builder(metricExporter).build();

var meterProvider = meterProviderBuilder.registerMetricReader(metricReader);
vishweshbankwar commented 1 year ago

Closing this issue for now. Will revisit post 1.6