open-telemetry / opentelemetry-dotnet

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

OTLP exporters: Support instantiating one channel/connection for sending traces, metrics, and logs #2426

Open alanwest opened 3 years ago

alanwest commented 3 years ago

Mentioned in this comment.

Probably most applicable to OTLP/gRPC but may also be relevant to think about for OTLP/HTTP.

Currently, instantiating an OTLP/gRPC exporter for traces, metrics and logs will cause a separate gRPC channel to be established for each telemetry signal. Each exporter has a constructor that takes OtlpExporterOptions which are use to construct the gRPC channel.

One thought is to allow creation of a channel from the OtlpExporterOptions and then pass the channel upon construction of the exporters.

Psuedo code:

var options = new OtlpExporterOptions { Endpoint = "http://localhost:4317" };
var channel = CreateOtlpGrpcChannel(options); // or perhaps an extension method on OtlpExporterOptions
var traceProvider = traceProviderBuilder.AddOtlpExporter(channel); // may need to pass the options as well
var meterProvider = meterProviderBuilder.AddOtlpExporter(channel);

Or maybe the channel becomes a property of the options and we pass the options when configuration the trace/meter providers.

public class OtlpExporterOptions
{
    public Channel GrpcChannel { get; } // Instantiates the channel on first access
    ...
}

var traceProvider = traceProviderBuilder.AddOtlpExporter(options);
var meterProvider = meterProviderBuilder.AddOtlpExporter(options);

A side note, OtlpExporterOptions may be utilized by both OTLP/gRPC and OTLP/HTTP exporters when #2316 lands, so it may make sense to not introduce gRPC specific things to OtlpExporterOptions or maybe refactor to have an OtlpGrpcExporterOptions/OtlpHttpExporterOptions, if we can.

reyang commented 3 years ago

One thought is to allow creation of a channel from the OtlpExporterOptions and then pass the channel upon construction of the exporters.

+1

I was thinking about something like this:

var transport = CreateTransport(...); var traceProvider = traceProviderBuilder.AddOtlpExporter(options => { options.Transport = transport; }).Build(); var meterProvider = meterProviderBuilder.AddOtlpExporter(options => { options.Transport = transport; }).Build();

The tricky part is that with transport being shared, the ownership and shutdown/forceflush semantic might need to be adjusted.

tillig commented 2 years ago

Just ran into something potentially related as I was setting up options, like

services
  .AddOptions<OtlpExporterOptions>()
  .Configure(opt => opt.Endpoint = myEndpoint);

This will end up setting the endpoint for both metrics and trace, but the spec appears to allow for separating the endpoints for metrics and trace. From an IOptions<T> pattern standpoint, it may be worth either...

CodeBlanch commented 1 year ago

Just FYI for anyone landing on here, as of 1.4.0 SDK named options work:

services.Configure<OtlpExporterOptions>("traces", opt => opt.Endpoint = myTraceEndpoint);
services.Configure<OtlpExporterOptions>("metrics", opt => opt.Endpoint = myMetricEndpoint);

tracerProviderBuilder.AddOtlpExporter(name: "traces", configure: null);
meterProviderBuilder.AddOtlpExporter(name: "metrics", configure: null);
github-actions[bot] commented 2 weeks ago

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