SceneQueryRunner: decouple time range comparisons #587
Conversation
| import { SceneObjectBase } from "../core/SceneObjectBase"; | ||
| import { SceneObjectState } from "../core/types"; | ||
|
|
||
| // A transformation function called by the query runner with responses |
There was a problem hiding this comment.
naming: we already have 'transformations', maybe this should have a different name...
| // | ||
| // The output is a single frame with the primary series and all transformed | ||
| // secondary series combined. | ||
| export const extraRequestProcessingOperator = (transforms: Map<string, TransformFunc>) => |
There was a problem hiding this comment.
same applies here re. naming of transforms
| } | ||
|
|
||
| // Indicates that this type wants to add extra requests to a query runner. | ||
| export interface SceneRequestAdder<T extends SceneObjectState> extends SceneObjectBase<T> { |
There was a problem hiding this comment.
I dislike the name SceneRequestAdder too, open to bikeshedding 🙂
sd2k
force-pushed
the
decouple-comparisons-from-queryrunner
branch
from
d35c5f3
to
7f75cef
Compare
|
@sd2k I think this looks interesting, sorry for late review. so much going on now with dashboard => scenes migration, and a customer project. We will try to review this in depth shortly, I think with some minor refactoring and name changes it looks overall promising / close, but I just did a very quick code scan |
No worries, it's not urgent, just wanted to make sure it didn't fall too far off the radar 👍 I'll try and add an example of how this can be used somewhere so it's a bit less abstract. |
sd2k
force-pushed
the
decouple-comparisons-from-queryrunner
branch
from
7f75cef
to
e12a606
Compare
|
FYI I've added an example of using this to the scenes-baseliner branch branch along with a demo, should be easy to run but you'll need to use a version of Grafana with grafana/grafana#82299 merged in to load the plugin correctly since it depends on a WASM module. I don't think that'd necessarily make it into the core repo since it's quite heavyweight but it's a decent example! Also a live demo of a custom application observability build with this enabled. |
|
One thing I bumped into while using this in other projects was that I can't see a way to turn off the extra I imagine the solution would be to have a separate embedded scene with the baseline controls or something, but it might be nice to be able to say 'this Edit: I just rebased on |
|
Hey @torkelo, do you think you (or someone else) will have time to take a look at this in the next week or so? Happy to walk through it if you like. |
sd2k
force-pushed
the
decouple-comparisons-from-queryrunner
branch
from
e12a606
to
4cc87f3
Compare
|
@sd2k sorry, it's a bit of a bad time now with G11 / GrafanaCon crunch. |
sd2k
force-pushed
the
decouple-comparisons-from-queryrunner
branch
4 times, most recently
from
8b74dd9
to
8154573
Compare
|
I've tried to come up with some better names for the things I wasn't happy with ('adder' because it kinda sucks, and 'transform' because it's overused). The new names are maybe slightly better? The best way to test this out, if anyone's interested, is to check out the Baselines demo in the demo app in the downstream |
sd2k
force-pushed
the
decouple-comparisons-from-queryrunner
branch
from
8154573
to
79d35df
Compare
Prior to this commit, `SceneQueryRunner` had special handling for `SceneTimeRangeCompare` objects, explicitly searching for them in the scene graph, adding additional queries, and transforming resulting queries. This made it difficult to re-use the general behaviour of 'running additional queries' in other objects. This commit introduces a new interface, `SceneRequestAdder`, which can be implemented to inform the query runner that it should run additional requests (returned by `getExtraRequests`) and transform the results in some fashion. Instead of searching the graph for `SceneTimeRangeCompare` objects, the query runner searches for implementors of `SceneRequestAdder` and uses those instead. The specifics of how it searches for these is a little bit fuzzy and should probably be improved: it walks up the graph until it finds at least one adder at the current level or in any children of the current level; adds any others at that level or in the children; then returns. `SceneTimeRangeCompare` has been refactored to make use of this new interface. I've also got a separate object which also implements it which is working well including when both are enabled.
sd2k
force-pushed
the
decouple-comparisons-from-queryrunner
branch
from
79d35df
to
4f58dee
Compare
Not sure these names are much better but they're possibly less ambiguous.
sd2k
force-pushed
the
decouple-comparisons-from-queryrunner
branch
from
4f58dee
to
9aa6700
Compare
Prior to this commit,
SceneQueryRunnerhad special handling forSceneTimeRangeCompareobjects, explicitly searching for them in thescene graph, adding additional queries, and transforming resulting
queries. This made it difficult to re-use the general behaviour of
'running additional queries' in other objects.
This commit introduces a new interface,
SceneRequestAdder, whichcan be implemented to inform the query runner that it should run
additional requests (returned by
getExtraRequests) and transform theresults in some fashion.
Instead of searching the graph for
SceneTimeRangeCompareobjects,the query runner searches for implementors of
SceneRequestAdderand uses those instead. The specifics of how it searches for these
is a little bit fuzzy and should probably be improved: it walks up the
graph until it finds at least one adder at the current level or in
any children of the current level; adds any others at that level
or in the children; then returns.
SceneTimeRangeComparehas been refactored to make use of this newinterface. I've also got a separate object which also implements
it which is working well including when both are enabled.
Relates to this Slack thread.