[Spec Compliance] OTLP Exporter Configuration Options

[utpilla]

Opening this to discuss the OTLP Exporter compliance issue

The spec for OTLP Exporter mentions these configuration options to be present: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md

OTLP Exporter options currently support these options:

• Endpoint
• GrpcChannelOptions (for netstandard2.1)/ Credentials and ChannelOptions (for != netstandard2.1)
• Headers

Issue1:

The configuration options that we have currently are gRPC specific.
For eg. the spec says that the options Headers could be used for both gRPC and HTTP. We have set the data type for Headers as Metadata which is from the Grpc client. If we add support for Http later we cannot use this option.

Issue2:

The user cannot provide the certificate file as an option
Currently, the user is expected to provide either the Credentials along with ChannelOptions for non-netstandard2.1, or the GrpcChannelOptions object with the HttpClientHandler which includes the user provided certificate. The spec says to add Certificate File as a configuration option. This would require require generating Credentials from the user provided Certificate for non-netstandard2.1 and creating the appropriate HttpClientHandler for netstandard2.1

Possible solutions to Issue 1

• Prefix the names of the options with the protocol name. For eg. Headers to be renamed to gRPCHeaders
• Have a different OTLP Exporter dedicated to each of the protocols

[cijothomas]

Another option is to trim the current public options to accept only generic options like a string Headers; instead of Metadata Headers, so that internally we can parse the string and produce appropriate options based on the underlying protocol.
(Similar for CertificatePath, and others)

[alanwest]

Have a different OTLP Exporter dedicated to each of the protocols

I wonder if we can get away with a single exporter but a different options class for each of the protocol i.e., HttpOtlpExporterOptions and GrpcOtlpExporterOptions. Then the exporter would be constructed with one or the other.

I think I like the idea of maintaining direct access to the underlying protocol options e.g. ChannelOptions, ChannelCredentials, Metadata. It maintains the most flexibility allowing consumers to have complete control over the channel's configuration.

[alanwest]

The user cannot provide the certificate file as an option

I actually have not played around with this. Though I had assumed that setting the credentials with the ChannelOptions (< netstandard2.1) or the HttpClientHandler (netstandard2.1) would be sufficient to meet the requirements of the spec. Are we thinking that this is not sufficient and that we need an explicit cert file option?

In order to support the OTEL_EXPORTER_OTLP_CERTIFICATE environment variable do we know if it's possible to generate the appropriate ChannelOptions?

[cijothomas]

I think I like the idea of maintaining direct access to the underlying protocol options e.g. ChannelOptions, ChannelCredentials, Metadata. It maintains the most flexibility allowing consumers to have complete control over the channel's configuration.

We can start with not exposing these, and based on needs, consider exposing more. If the requirement is not .NET specific, we can get it updated in the spec.

[cijothomas]

#1781 Makes Headers generic enough, and in compliance with spec. and also added timeout as per spec.
Endpoint was already available. (As Uri for netstandard2.1. I think we can make it string Endpoint for both, and internally generate Uri, just like headers).

I propose the following:
Remove ChannelOptions and ChannelCredentials from options for 1.0 Release.
Replace Endpoint as string (instead of Uri) for netstandard2_1
Leave Compression as unsupported.
This achieves a closer spec compliance (with exception of cert, compression), and also we will have the ability to add more capabilities in a backward compatible way in next release.

@alanwest @CodeBlanch @pjanotti @reyang @SergeyKanzhelev Please share any concerns with this. (This is the last blocker for 1.0)

[alanwest]

I agree with the approach.

Replace Endpoint as string (instead of Uri) for netstandard2_1

One thing to keep in mind here is that the Grpc.Net.Client library (netstandard2.1) requires the scheme to be declared e.g., https://localhost:4317. The other gRPC library is ok with just declaring localhost:4317. We can probably just document this expectation.

[CodeBlanch]

If we remove ChannelOptions & ChannelCredentials will still be able to do SSL? I'm not too familiar with the google client.

[alanwest]

If we remove ChannelOptions & ChannelCredentials will still be able to do SSL? I'm not too familiar with the google client.

We need to vet this out, but my thought was that if someone provided a cert file via configuration then we'd set the ChannelCredentials appropriately.

Here are some examples https://grpc.io/docs/guides/auth/#csharp

Seems the spec has conceived of the first two variations

• https://grpc.io/docs/guides/auth/#base-case---no-encryption-or-authentication-3
• https://grpc.io/docs/guides/auth/#with-server-authentication-ssltls-3

[pjanotti]

Leave Compression as unsupported

IIRC it should be possible to enable it via metadata - that said I think it is fine for a client library to not have this capability.

Remove ChannelCredentials

As already noted by @alanwest and @CodeBlanch will block SSL. If we can just choose between insecure and new SslCredentials() it should be enough: IIRC then defaults to the trusted store and has and supports an env var for custom ones.

[alanwest]

I think we could cover all the cases if the logic were something like

Google gRPC library:

if (cfg.Endpoint.StartsWith("https") && cfg.CertificateFile == null)
{
    var channel = new Channel(cfg.Endpoint, new SslCredentials());
}
else if (cfg.Endpoint.StartsWith("https") && cfg.CertificateFile != null)
{
    var channelCredentials = new SslCredentials(File.ReadAllText(cfg.CertificateFile));
    var channel = new Channel(cfg.Endpoint, channelCredentials);
}
else
{
    var channel = new Channel(cfg.Endpoint, ChannelCredentials.Insecure);
}

Grpc.Net.Client (netstandard2.1)

// see https://docs.microsoft.com/aspnet/core/grpc/authn-and-authz?view=aspnetcore-5.0#client-certificate-authentication
if (cfg.CertificateFile != null)
{
    var certificate = ReadTheFile(cfg.CertificateFile); // Probably a way to do this...

    var handler = new HttpClientHandler();
    handler.ClientCertificates.Add(certificate);

    var channel = GrpcChannel.ForAddress(cfg.Endpoint, new GrpcChannelOptions
    {
        HttpHandler = handler
    });
}
else
{
    var channel = GrpcChannel.ForAddress(cfg.Endpoint)
}

---

Update

I had some wrong assumptions regarding the Google gRPC library. The channel cannot be constructed with a URI that includes the http/https scheme. So something like the following should work

if (cfg.CertificateFile != null)
{
    var channelCredentials = new SslCredentials(File.ReadAllText(cfg.CertificateFile));
    var channel = new Channel(cfg.Endpoint, channelCredentials);
}
else
{
    var channel = new Channel(cfg.Endpoint, ChannelCredentials.Insecure);
}

However, I'm uncertain whether it should be possible to use a secure channel without the need for supplying a certificate file. If this is possible, then it seems to me the specification is insufficient to allow this.

[pjanotti]

Sorry if I'm missing some context but why not simply delegate to the user, something like (hand compiled code):

                        .AddOtlpExporter(otlpOptions =>
                        {
                            otlpOptions.Endpoint = new Uri(this.Configuration.GetValue<string>("Otlp:Endpoint"));
                            otlpOptions.GrpcChannelOptions.Credentials = new Grpc.Core.SslCredentials();
                        }));

[cijothomas]

@pjanotti One of the goal is to avoid Grpc specific things from the Options. That means removing GrpcChannelOptions from otlpOptions. Instead just expose endpoint, certFile in options. (matching spec)

[alanwest]

Sorry if I'm missing some context but why not simply delegate to the user, something like (hand compiled code):

@pjanotti That's effectively what we have now. The concern is that, in your example, GrpcChannelOptions.Credentials is not part of the spec. The spec has a narrow set of config options https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#configuration-options

😄 Cijo beat me...

[cijothomas]

Spoke offline with Reiley, Alan about this more and we need to take more time to explore the best way to handle OTLPExporterOptions.
Several options are available, including - Keep as is (i.e expose grpc classes in public api), expose only string like options (from spec) and handle everything internally, a combination of both, or ConnectionString approach which allows easy extensibility.

To keep up the original plan of releasing 1.0 this week (spec is marked stable, so we are not blocked from there), here are our options:

1. Release 1.0 without OTLPExporter. Followed by a separate OTLPExporter 1.0 release once the above issue is resolved.
2. Release 1.0 with OTLPExporter which do not support ChannelOptions/ChannelCredentials. (this means users cannot use TLS connections).. And follow with 1.1 release once the feature is added.

[CodeBlanch]

If we make Endpoint a Uri could we do something like this:

var channel;
if (cfg.Endpoint.Scheme == Uri.UriSchemeHttps)
{
   if (cfg.CertificateFile == null)
   {
     channel = new Channel(cfg.Endpoint.Authority, new SslCredentials());
   }
   else
   {
     channel = new Channel(cfg.Endpoint.Authority, new SslCredentials(File.ReadAllText(cfg.CertificateFile)));
   }
}
else
{
    channel = new Channel(cfg.Endpoint.Authority, ChannelCredentials.Insecure);
}

I think allowing for the new SslCredentials() case is needed because you might not want to use a cert file but still use SSL?

[CodeBlanch]

@cijothomas I am good with either 1 or 2... leaning towards 2.

[pjanotti]

I prefer option 2, @CodeBlanch suggestion above is good.

[alanwest]

I think allowing for the new SslCredentials() case is needed because you might not want to use a cert file but still use SSL?

Yep, this is the use case we're trying to sort out. I think there's a little more to it than just a check like if (cfg.Endpoint.Scheme == Uri.UriSchemeHttps). That is, you can do gRPC over unix sockets with TLS as well.

Ideally I think we go with option 2 giving us some time to push for some clarification with the spec.

[cijothomas]

@alanwest We can close this issue right? we currently support everything 1.0 spec requires.

[alanwest]

@cijothomas We do not currently support the option to set a certificate file. Might make sense to capture this in an issue of its own and close this one. Also, we don't currently support configuring the OTLP exporter via environment variables - I thought we had an issue for this... not finding it off hand.

[cijothomas]

#1453 - issue for tracking ENV var based config for overall. Dont have one for each component, but this should suffice.

#2009 Cert issue.

Will close this one.