otlpHTTPLogExporter, otlpGRPCLogExporter, etc do not support fields like OTLP.Certificate

mattsains commented 6 days ago

Description

The OTLP configuration type has fields Certificate, ClientCertificate, ClientKey, HeadersList and Insecure:

https://github.com/open-telemetry/opentelemetry-go-contrib/blob/6e79f7ca22d58345e2ccfe3f9e8bb4ad71633ab3/config/generated_config.go#L258-L264

These fields are not read/supported by constructors like otlpGRPCLogExporter, otlpHTTPLogExporter, otlpHTTPMetricExporter, otlpGRPCMetricExporter, otlpGRPCSpanExporter and otlpHTTPSpanExporter.

Environment

OS: N/A
Architecture: N/A
Go Version: N/A
config version: 6e79f7c, 1.32.0

Steps To Reproduce

An example of how to trigger this is to call NewSDK specifying a custom certificate. This certificate will not be used when connecting to the trace endpoint specified.

Expected behavior

These fields should be respected when creating log, trace and metric exporters.

mattsains commented 6 days ago

I think that this code is used in the collector's internal telemetry exporting too: https://github.com/open-telemetry/opentelemetry-collector/blob/9d2685f404ef65b46152ae80c36ff84f303e3cc6/service/service.go#L133C5-L138C7

I believe the fix for this is as simple as something like this case for Certificate:

if otlpConfig.Certificate != nil {
    creds, err := credentials.NewClientTLSFromFile(*otlpConfig.Certificate, "")
    opts = append(opts, otlptracegrpc.WithTLSCredentials(creds))
}

If that sounds good to maintainers, I'd be glad to submit a PR for it, I'd just like to know I'm barking up the right tree first.

ms-jcorley commented 6 days ago

I created a PR for a small part of this in the collector repo: https://github.com/open-telemetry/opentelemetry-collector/pull/11633

dmathieu commented 6 days ago

cc @pellared @codeboten as code owners.

open-telemetry / opentelemetry-go-contrib