OTLP/JSON: span status.code written and parsed as short enum name instead of integer
- Dominant language
- C
- Stars
- 8.1k
- Forks
- 2k
- Avg merge
- 4d 20h
- Merged PRs (30d)
- 71
Description
## Bug Report
**Describe the bug**
In OTLP/JSON, fluent-bit encodes and decodes the span `status.code` as a short enum
name (`UNSET` / `OK` / `ERROR`). The OTLP specification requires enum fields to be
encoded as **integers**, and the short names are not valid protobuf enum names either
(those would be `STATUS_CODE_UNSET` / `STATUS_CODE_OK` / `STATUS_CODE_ERROR`).
The result is that fluent-bit's OTLP/JSON is only interoperable with fluent-bit:
* what it **writes** is rejected by spec-compliant consumers,
* what a spec-compliant producer **writes** is rejected by fluent-bit.
We hit this while using fluent-bit as a log sink in CI: an `opentelemetry` input
receives the OTLP protobuf traffic of the machine under test and an `stdout` output
with `format: otlp_json` writes it to the journal, so the test suite can read the
telemetry back. Every span that carries a status is silently dropped by the reader,
because the OTLP/JSON unmarshaler rejects the whole record.
**To Reproduce**
Config:
```yaml
service:
flush: 1
log_level: info
pipeline:
inputs:
- name: opentelemetry
listen: 0.0.0.0
port: 4318
tag: sink.traces
outputs:
- name: stdout
match: sink.traces
format: otlp_json
```
*Encoding side* - send a span with a status (OTLP/protobuf, as any SDK does) and
fluent-bit prints:
```json
{"resourceSpans":[{...,"spans":[{...,"status":{"code":"UNSET","message":""}}]}]}
{"resourceSpans":[{...,"spans":[{...,"status":{"code":"ERROR","message":"boom"}}]}]}
```
Feeding that into any pdata-based consumer (OpenTelemetry Collector
`otlpjsonfilereceiver`, `otel-tui`) drops the record without a message.
*Decoding side* - post OTLP/JSON to the same input, only the non-conformant spelling
is accepted:
```
"status":{"code":2} -> HTTP 400, "invalid JSON trace: conversion error (status: 43)"
"status":{"code":"STATUS_CODE_ERROR"} -> HTTP 400, "invalid JSON trace: conversion error (status: 43)"
"status":{"code":"ERROR"} -> HTTP 201
```
**Expected behavior**
Per
[opentelemetry-proto/docs/specification.md](https://github.com/open-telemetry/opentelemetry-proto/blob/main/docs/specification.md):
> Values of enum fields MUST be encoded as integer values. Unlike the standard
> Protobuf JSON Mapping, which allows values of enum fields to be encoded as either
> integer values or as enum name strings, only integer enum values are allowed in
> OTLP JSON Protobuf Encoding; the enum name strings MUST NOT be used.
So the encoder should write `"code": 2`, and the decoder should accept integers.
**Where in the code**
Encoder - `src/opentelemetry/flb_opentelemetry_otlp_json.c` (`create_trace_status_json`):
```c
if (status->code == CTRACE_SPAN_STATUS_CODE_OK) {
code_string = "OK";
}
else if (status->code == CTRACE_SPAN_STATUS_CODE_ERROR) {
code_string = "ERROR";
}
else {
code_string = "UNSET";
}
flb_json_mut_obj_add_str(doc, json, "code", code_string);
```
Decoder - `src/opentelemetry/flb_opentelemetry_traces.c`: the `code` entry is only
read when it is a `MSGPACK_OBJECT_STR`, and only `UNSET` / `OK` / `ERROR` are mapped;
anything else returns `-1`. An integer value is not handled at all.
**Your Environment**
* Version used: 5.0.6 and 5.1.2, both reproduce (official `fluent/fluent-bit` images)
* Configuration: see above
* Environment name and version: Docker, `--network host`
* Server type and version: n/a
* Operating System and version: Linux
* Filters and plugins: `in_opentelemetry`, `out_stdout` with `format: otlp_json`
**Anything else / question before we send a PR**
We would like to fix this, but the fix is a breaking change for anyone whose pipeline
currently has fluent-bit on both ends of an OTLP/JSON hop: the encoder and the decoder
are consistent with each other today, so switching the encoder to integers without
touching the decoder would break those setups.
Our proposal would be:
1. encoder writes the integer value, as the spec requires,
2. decoder accepts integers, and keeps accepting the current short names (and
optionally the canonical `STATUS_CODE_*` names) so existing fluent-bit to
fluent-bit pipelines keep working.
Would you take a PR along those lines, or do you prefer the change to be gated behind
a configuration option? Happy to follow whichever direction you consider right.
Contributor guide
Research direction
Start in src/opentelemetry/flb_opentelemetry_otlp_json.c at create_trace_status_json and in src/opentelemetry/flb_opentelemetry_traces.c where the status.code entry is decoded. Trace the existing integer and short-name handling, then verify that encoding produces integer status codes and decoding accepts integers while preserving the current short-name compatibility described in the issue.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- observability-sre
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100