Exporter signal-specific environment variables interpretation

[cbandy]

Description

Spec v1.8.0 clarified that per-signal environment variables for HTTP endpoints must be interpreted as full/complete URLs.

otlptracehttp@v1.2.0 (incorrectly) appends /v1/traces to the URL provided in OTEL_EXPORTER_OTLP_TRACES_ENDPOINT.

Environment

• Go Version: 1.16, 1.17
• opentelemetry-go version: v1.1.0, v1.2.0

Steps To Reproduce

package main

import (
	"context"
	"fmt"
	"net"
	"net/http"

	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp"
)

func main() {
	l, err := net.Listen("tcp", "localhost:9999")
	if err != nil {
		panic(err)
	}
	defer l.Close()

	server := http.Server{}
	server.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		fmt.Println("URL:", r.URL)
	})
	go server.Serve(l)

	client := otlptracehttp.NewClient()
	client.Start(context.Background())
	fmt.Println("err:", client.UploadTraces(context.Background(), nil))
}

$ OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:9999 go run main.go 
URL: /v1/traces
err: <nil>

$ OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=http://localhost:9999 go run main.go 
URL: /v1/traces
err: <nil>

$ OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:9999/asdf go run main.go 
URL: /asdf/v1/traces
err: <nil>

$ OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=http://localhost:9999/asdf go run main.go 
URL: /asdf/v1/traces
err: <nil>

Expected behavior

I expected OTEL_EXPORTER_OTLP_TRACES_ENDPOINT to "be used as-is without any modification".

That is, I expected the OTEL_EXPORTER_OTLP_ENDPOINT env var to be interpreted differently than the OTEL_EXPORTER_OTLP_TRACES_ENDPOINT env var.

[cbandy]

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#endpoint-urls-for-otlphttp

1. For the per-signal variables (OTEL_EXPORTER_OTLP_<signal>_ENDPOINT), the URL MUST be used as-is without any modification. The only exception is that if an URL contains no path part, the root path / MUST be used (see Example 2).

[cbandy]

Same results in otlptracehttp@v1.2.0.

[arielvalentin]

I believe the URL path is being overridden here:

opentelemetry-go/exporters/otlp/otlptrace/otlptracehttp/client.go

Line 91 in f4b63f1

if tmp == "" {

This should check if the signal specific endpoint was defined, however the existing patterns seem to evaluate environment variables and set defaults in a different function:

opentelemetry-go/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go

Line 83 in f4b63f1

if v, ok := e.getEnvValue("TRACES_ENDPOINT"); ok {

@open-telemetry/go-maintainers Would it be best to separate the protocol, host and port from the path here and update the URL path like it does with Secure/Insecure?

[MrAlias]

Looks like the HTTP client will always append a suffix:

opentelemetry-go/exporters/otlp/otlptrace/otlptracehttp/client.go

Line 204 in 2b7c650

address := fmt.Sprintf("%s://%s%s", d.getScheme(), d.cfg.Endpoint, d.cfg.URLPath)

And the gRPC client will never:

opentelemetry-go/exporters/otlp/otlptrace/otlptracegrpc/client.go

Line 73 in 2b7c650

endpoint: cfg.Traces.Endpoint,

If we update the otlpconfig package to appropriately parse the endpoint based on the source and update the HTTP client to not modify the endpoint it should resolve this.