opentelemetry-go icon indicating copy to clipboard operation
opentelemetry-go copied to clipboard

OTLP exporters should not percent decode the key when parsing HEADERS env vars

Open pellared opened this issue 1 year ago • 3 comments

The OTLP exporters SHOULD NOT percent decode the header keys when parsing OTEL_EXPORTER_OTLP_HEADERS, OTEL_EXPORTER_OTLP_TRACES_HEADERS, OTEL_EXPORTER_OTLP_METRICS_HEADERS, OTEL_EXPORTER_OTLP_LOGS_HEADERS env vars. Moreover, only valid header keys should be parsed.

Current behavior:

https://github.com/open-telemetry/opentelemetry-go/blob/29bdfd20e07a156f22a9c9148066aae6527f8219/exporters/otlp/otlptrace/otlptracegrpc/internal/envconfig/envconfig.go#L166-L170

The specification says https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#specifying-headers-via-environment-variables:

The OTEL_EXPORTER_OTLP_HEADERS, OTEL_EXPORTER_OTLP_TRACES_HEADERS, OTEL_EXPORTER_OTLP_METRICS_HEADERS, OTEL_EXPORTER_OTLP_LOGS_HEADERS environment variables will contain a list of key value pairs, and these are expected to be represented in a format matching to the W3C Baggage, except that additional semi-colon delimited metadata is not supported, i.e.: key1=value1,key2=value2. All attribute values MUST be considered strings.

From W3C Baggage spec https://www.w3.org/TR/baggage/#key:

A token which identifies a value in the baggage. token is defined in RFC7230, Section 3.2.6. Leading and trailing whitespaces (OWS) are allowed and are not considered to be a part of the key.

NOTE Though the baggage header is a [UTF-8] encoded [UNICODE] string, key is limited to the ASCII code points allowed by the definition of token in [RFC7230]. This is due to the implementation details of stable implementations prior to the writing of this specification.

There is nothing about percent encoding. Only the values are supposed to be percent decoded.

Notice that the implementation should also make sure that only valid key characters are used.

pellared avatar Jul 16 '24 06:07 pellared

I will open a PR for it...

zhihali avatar Aug 10 '24 09:08 zhihali

the OTLPlog should be updated as well, I have raised an issue related https://github.com/open-telemetry/opentelemetry-go/issues/5722, and will implement a fix soon

zhihali avatar Aug 20 '24 13:08 zhihali

@zhihali, this is for all OTLP exporters. I see that OTLP trace and metric exporters are already fixed. Therefore, only OTLP log exporters need bugfixes.

pellared avatar Sep 05 '24 09:09 pellared

@pellared , @MrAlias , I have raised a PR for OTLP log exporter bugfixes. Please review. Thanks

tongoss avatar Mar 02 '25 06:03 tongoss

Resolved by #6392

MrAlias avatar Mar 26 '25 19:03 MrAlias