search

grafana / grafana

The open and composable observability and data visualization platform. Visualize metrics, logs, and traces from multiple sources like Prometheus, Loki, Elasticsearch, InfluxDB, Postgres and many more.
https://grafana.com
GNU Affero General Public License v3.0
61.15k stars 11.68k forks source link

Tempo: Don't modify the passed time range when using timeShiftEnabled #87980

Closed aocenas closed 3 weeks ago

aocenas commented 1 month ago

Fixes: https://github.com/grafana/grafana/issues/87608

The moment methods to modify the date return an object so it seems like they should be immutable but actually also modify the date object. So if the object is passed for example from Explore we modify the explore state as well without going through the correct redux updates which creates issues.

aocenas commented 3 weeks ago

@joey-grafana

And do we want to do that? Like there are 2 cases

  1. clicking from the traces search to open a single trace through data link. In that case it should have been implemented inside the data link and not where it is right now
  2. we seem to apply this for traceID type of query just for sure I guess, to make sure user gets a a full trace. In this case it should not change the time range, like then every time you run a query the time range gets wider which imho seems pretty odd and does not actually help (because without knowing you are making the query slower without added benefit).

And then if we want and have 2. then I don't think we need 1.

aocenas commented 3 weeks ago

request.range is updated but maybe we also need to update the raw value in request.range. I've also noticed there's a request.rawRange which I'm not sure why we're doing this!

Although the UI is not being updated so maybe it's a Redux state/update issue.

I am not sure I understand this but this is not how redux works. Like updating an object value directly that is part of state whether it's redux or react state is a bug (unless it's a Ref type). You create a weird undefined behaviour as you are skipping any rerendering or onChange hooks and reducers etc.