Show stream labels in context modal#673
Conversation
…stom - Parse `_stream` per row into a label->value map and ship it, together with the per-row `_stream_id`, in frame meta.custom for the "Show context" stream label selector - Drop `_stream` from the log labels now that it is carried in meta.custom - Apply to both the instant and streaming response paths
… configurable size)
- Refactor datasource class, move out the LogContextProvider in the separate class
- Render each stream label of the log row as a toggleable chip; toggling a
label off widens the context search from `_stream_id` to a `_stream:{...}`
selector built from the still-enabled labels
Loori-R
left a comment
Loori-R
left a comment
There was a problem hiding this comment.
Looks good overall. I left a few questions.
| value.Del(streamField) | ||
| stf, err := utils.ParseStreamFields(rowStream) | ||
| if err != nil { | ||
| return newResponseError(fmt.Errorf("error parse _stream field: %s", err), backend.StatusInternal) |
There was a problem hiding this comment.
Invalid _stream now fails the whole logs response. Since it is only needed for the context selector, can we skip meta.custom.streams for this row instead?
There was a problem hiding this comment.
I just partially reverted the changes as it was early - https://github.com/VictoriaMetrics/victorialogs-datasource/blob/v0.28.0/pkg/plugin/response.go#L122 . We also threw an error.
IMO, the _stream field should always be in the log. So if something is wrong with the _stream field, it will be better to throw an error. So it could be fixed.
| // parse `_stream` into a per-row label map for the log context UI and | ||
| // drop it from the row so it does not show up among the log labels | ||
| rowStream := string(value.GetStringBytes(streamField)) | ||
| value.Del(streamField) |
There was a problem hiding this comment.
Does this code remove the _stream field from the response? If yes, could this break existing Explore/table views, field overrides, or Grafana transformations that reference _stream?
There was a problem hiding this comment.
It shouldn't affect the backward compatibility, as in the current release, this field is also removed on the backend side.
| getRowStreamId = (row: LogRowModel): string => { | ||
| const streamIds = row.dataFrame.meta?.custom?.streamIds; | ||
| if (streamIds && streamIds.length > 0 && streamIds[row.rowIndex]) { | ||
| return streamIds[row.rowIndex]; | ||
| } | ||
|
|
||
| return ''; | ||
| }; |
There was a problem hiding this comment.
The previous _stream_id fallback from row.labels was removed here. Is that intentional? If meta.custom.streamIds is missing, the context query now uses _stream_id:"".
There was a problem hiding this comment.
Good point!
But I think that from the DS backend we should get streamId every time and need to move this logic to the DS backend.
I thought that _stream_id should always be. But also need to handle edge cases correctly where the user manually removed system fields (_stream, _stream_id) via a query.
2 participants
Related issue: #665
Describe Your Changes
Context for all pods:
Checklist
The following checks are mandatory:
Summary by cubic
Show stream labels in the “Show context” modal so you can widen context by toggling labels and view logs across pods/services. Backend parses
_streamand sends per‑row labels/ids viameta.customto power the selector. Addresses #665.New Features
_stream_idby default, or_stream:{...}from enabled labels when some are toggled off.Refactors
_streamper row intometa.custom.streamsandmeta.custom.streamIds; remove_streamfrom log labels.LogContextProvider; addedLogContextUIand a sharedToggleChipcomponent.meta.customintact in frame processing, passing throughstreams/streamIds.Written for commit f17238a. Summary will update on new commits.