open-telemetry/opentelemetry-go

OTLP Log gRPC ignores OTEL_EXPORTER_OTLP_INSECURE=true

jpkrohling opened this issue · 3 comments

Context:

When setting OTEL_EXPORTER_OTLP_INSECURE=true, this is ignored, as the envInsecure is using envEndpoint, when it should use something like:

	envInsecure = []string{
		"OTEL_EXPORTER_OTLP_LOGS_INSECURE",
		"OTEL_EXPORTER_OTLP_INSECURE",
	}

Hi there!
The current behavior makes sense, as stated in the specification:

Insecure: Whether to enable client transport security for the exporter’s gRPC connection. This option only applies to OTLP/gRPC when an endpoint is provided without the http or https scheme - OTLP/HTTP always uses the scheme provided for the endpoint. Implementations MAY choose to not implement the insecure option if it is not required or supported by the underlying gRPC client implementation.

So when OTEL_EXPORTER_OTLP_ENDPOINT is passed, we use Insecure=false if the scheme is https, and Insecure=true otherwise.

func convInsecure(s string) (bool, error) {

I want to draw attention to the fact that the value of this environment variable must include a schema (or look like //url.without.scheme:8080 for urls without scheme). This is because we use url.Parse, which states:

Trying to parse a hostname and path without a scheme is invalid but may not necessarily return an error, due to parsing ambiguities.
(Probably that should be stated more explicitly)

Additionally, I'm not sure if it is correct behavior that if we create an exporter with these settings:
otlploggrpc.New(nil, otlploggrpc.WithEndpointURL("https://some.url:8080"), otlploggrpc.WithInsecure())
We will get an insecure connection with an https endpoint, because WithInsecure goes after WithEndpointURL.

OTEL_EXPORTER_OTLP_ENDPOINT is not provided in my case, I'm relying on the default value. Note that the behavior is correct for traces and metrics.

Yeah, you are right :)
I just wanted to point out that the fix could be a bit tricky and not just a simple change to envInsecure.
Anyway, I can work on it.