Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support OTLP_HEADERS with capital case #1851

Closed
v1v opened this issue May 14, 2021 · 5 comments · Fixed by #1869
Closed

Support OTLP_HEADERS with capital case #1851

v1v opened this issue May 14, 2021 · 5 comments · Fixed by #1869
Assignees
Labels
bug Something isn't working

Comments

@v1v
Copy link

v1v commented May 14, 2021

Describe your environment

OS: Ubuntu 20
Python version: python3.8
Dependencies installed with venv:

Installing collected packages: six, protobuf, opentelemetry-semantic-conventions, opentelemetry-api, opentelemetry-sdk, opentelemetry-proto, grpcio, googleapis-common-protos, backoff, opentelemetry-exporter-otlp-proto-grpc, opentelemetry-exporter-otlp
Successfully installed backoff-1.10.0 googleapis-common-protos-1.53.0 grpcio-1.37.0 opentelemetry-api-1.1.0 opentelemetry-exporter-otlp-1.1.0 opentelemetry-exporter-otlp-proto-grpc-1.1.0 opentelemetry-proto-1.1.0 opentelemetry-sdk-1.1.0 opentelemetry-semantic-conventions-0.20b0 protobuf-3.15.8 six-1.15.

Steps to reproduce

Create a OTLP Header with a capital case format:

export OTEL_EXPORTER_OTLP_HEADERS="Authorization=Bearer apm_secret_token"

What is the expected behavior?

Allow to use headers with the first Capital letter

What is the actual behavior?

E0427 12:07:17.398505136    3317 call.cc:919]                validate_metadata: {"created":"@1619525237.398491209","description":"Illegal header key","file":"src/core/lib/surface/validate_metadata.cc","file_line":44,"offset":0,"raw_bytes":"41 75 74 68 6f 72 69 7a 61 74 69 6f 6e 'Authorization'\u0000"}
Exception while exporting Span batch.
Traceback (most recent call last):
  File "/tmp/workspace/ansible-plugin/.venv/lib/python3.8/site-packages/opentelemetry/sdk/trace/export/__init__.py", line 333, in _export_batch
    self.span_exporter.export(self.spans_list[:idx])  # type: ignore
  File "/tmp/workspace/ansible-plugin/.venv/lib/python3.8/site-packages/opentelemetry/exporter/otlp/proto/grpc/trace_exporter/__init__.py", line 294, in export
    return self._export(spans)
  File "/tmp/workspace/ansible-plugin/.venv/lib/python3.8/site-packages/opentelemetry/exporter/otlp/proto/grpc/exporter.py", line 261, in _export
    self._client.Export(
  File "/tmp/workspace/ansible-plugin/.venv/lib/python3.8/site-packages/grpc/_channel.py", line 944, in __call__
    state, call, = self._blocking(request, timeout, metadata, credentials,
  File "/tmp/workspace/ansible-plugin/.venv/lib/python3.8/site-packages/grpc/_channel.py", line 926, in _blocking
    call = self._channel.segregated_call(
  File "src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi", line 498, in grpc._cython.cygrpc.Channel.segregated_call
  File "src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi", line 366, in grpc._cython.cygrpc._segregated_call
  File "src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi", line 360, in grpc._cython.cygrpc._segregated_call
  File "src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi", line 218, in grpc._cython.cygrpc._call
  File "src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi", line 253, in grpc._cython.cygrpc._call
  File "src/python/grpcio/grpc/_cython/_cygrpc/channel.pyx.pxi", line 61, in grpc._cython.cygrpc._raise_call_error
ValueError: metadata was invalid: ('Authorization', 'Bearer ****')

Additional context

See elastic/apm-server#5169

@v1v v1v added the bug Something isn't working label May 14, 2021
@eddyleelin
Copy link
Contributor

eddyleelin commented May 14, 2021

Just to clarify, it seems like the comment in the linked issue is suggesting that the entire header key (eg. "Authorization" or "AUTHORIZATION") should be cast to lower case ("authorization")?

The otel specifications for OTLP exporters seems to support using all lower-case to match the format of headers in W3C Correlation-Context:

In order to increase interoperability across multiple protocols and encourage successful integration, implementations SHOULD keep the header name lowercase.

The line in question seems to be here?

@eddyleelin
Copy link
Contributor

I could fix this, if it's valid! @owais @codeboten

@owais
Copy link
Contributor

owais commented May 16, 2021

I don't think the exporter enforces anything here. IIRC, gRPRC expects all metadata (headers) to be lowercase and that's where the error is coming from.

@owais
Copy link
Contributor

owais commented May 16, 2021

We could convert all headers to lower case in order to prevent the crash and make it work with gRPC.

@lzchen
Copy link
Contributor

lzchen commented May 18, 2021

@owais
I think that is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants