search

grafana / tempo

Grafana Tempo is a high volume, minimal dependency distributed tracing backend.
https://grafana.com/oss/tempo/
GNU Affero General Public License v3.0
4.03k stars 522 forks source link

tempo-cli: add support for /api/v2/traces endpoint #4127

Closed electron0zero closed 1 month ago

electron0zero commented 1 month ago

What this PR does: Use /api/v2/traces by default to query trace by id and get partial traces for bigger traces, also add --v1 flag in tempo-cli query api trace-id command to query /api/traces endpoint.

see https://github.com/grafana/tempo/pull/3941 for more details on /api/v2/traces.

example usage: tempo-cli query api trace-id http://tempo:3200 fd9b20c52c74f67de6a69279bfba3936 --v1 > trace.json

there is a change in default behaviour and you need to use --v1 if you are on a older version of tempo that doesn't support /api/v2/traces or want to use /api/traces, because of that, this change is marked as BREAKING CHANGE

This PR also includes a small change in the way we Marshal the trace.

encoding/json package can't handle +Inf, -Inf, NaN values and will fail to Marshal the response from tracebyid endpoints if response has keys with values of +Inf, -Inf or NaN.

gogo/protobuf/jsonpb package correctly handles these values so use that to Marshal and print the response to stdout

Checklist

joe-elliott commented 1 month ago

Should we just make v2 default? and require a flag to use v1? We could list it as a breaking change if we feel it needs to be.

electron0zero commented 1 month ago

Should we just make v2 default? and require a flag to use v1? We could list it as a breaking change if we feel it needs to be.

we can yes, I left --v2 under a flag assuming not everyone is running latest tempo with /api/v2/traces so it might break the CLI for them but these users can still use the older version of tempo-cli and if they upgarde they can use --v1 flag

will flip the flag to be --v1 and make --v2 default and mark it as breaking change.

electron0zero commented 1 month ago

Should we just make v2 default? and require a flag to use v1? We could list it as a breaking change if we feel it needs to be.

updated, @joe-elliott v1 is behind flag, and v2 is now default.