[PR] Give every span of an OTLP trace response a status [skywalking]

["wankai123 (via GitHub)" <gi...@apache.org>]

wankai123 opened a new pull request, #14100:
URL: https://github.com/apache/skywalking/pull/14100

   ### Fix the `/otlp` TraceQL trace view failing in Grafana with `runtime error: invalid memory address or nil pointer dereference`
   - [x] Add a unit test to verify that the fix works.
   - [x] Explain briefly why the bug exists and how to fix it.
   
   **Why the bug exists.** `status` is an optional field of an OTLP span, and senders do leave it out. APISIX's `opentelemetry` plugin, for one, sets a status only when the upstream answered 5xx:
   
   ```lua
   if upstream_status and upstream_status >= 500 then
       span:set_status(span_status.error, "upstream response status: " .. upstream_status)
   ```
   
   Grafana's Tempo datasource reads the field without checking that it is there, in `pkg/tempo/trace_transform.go`:
   
   ```go
   status := span.Status
   statusCode := int64(status.Code)
   ```
   
   So a trace containing one such span crashes Grafana's trace view: the plugin panics, `/api/ds/query` answers 500, and the user sees "Internal Server Error - please inspect Grafana server log for details". The panic stack runs `tempo.go` → `trace.go` → `trace_transform.go`, the trace-by-id path only; search is unaffected because its response carries no span status. The code is unguarded in Grafana 12.4.2, in the latest 12.4 patch, and in the extracted `grafana/grafana-tempo-datasource` that 13.x uses, so upgrading Grafana does not avoid it.
   
   This surfaced now because an OpenTelemetry Collector used to hide it. Its internal model holds the status as a value rather than a pointer, so every span it re-serialized carried at least an empty status. A span stored while a collector was in the path reads back as `status={}`; the same span sent straight to the OAP has the field absent.
   
   **The fix.** `OTLPTraceAssembler.assemble` already rebuilds every span while it regroups them by resource and scope, so it now fills in an empty status when the stored span has none. An absent status and an empty one both mean `STATUS_CODE_UNSET`, so nothing an API client reads changes. Because this happens on read, traces already in storage render too. The SkyWalking and Zipkin datasources set a status on every span they convert, so only the native OTLP path needed the guard.
   
   **Tests.** Two new cases in `OTLPTraceAssemblerTest`: a span stored without a status comes back with `STATUS_CODE_UNSET` and the field present in the JSON encoding as well, and a span stored with an error status keeps it untouched. One assertion in `shouldRegroupByResourceThenScopeAndOrderSpansByStart` compared an assembled span byte for byte against its stored form and now expects the filled-in status; the whole module passes at 58 tests.
   
   **Verified** on a Kubernetes deployment of the showcase whose OpenTelemetry-instrumented services export straight to the OAP. A trace whose APISIX span had no status went from `/api/ds/query` returning 500 with a panic in Grafana's log, to 200 with all 27 span rows and no panic, without re-ingesting anything.
   
   - [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. No issue is filed; found while running the showcase without a collector in the trace path.
   - [ ] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/docs/en/changes/changes.md).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Give every span of an OTLP trace response a status [skywalking]

["wankai123 (via GitHub)" <gi...@apache.org>]

wankai123 merged PR #14100:
URL: https://github.com/apache/skywalking/pull/14100


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org