WIP/Idea: Streaming: add common request options #18728
Conversation
| * | ||
| * NOTE: optional now, but should be required | ||
| */ | ||
| shape?: StreamingResponseShape; |
There was a problem hiding this comment.
Can't we infer it from the data? Like see the range and compare with existing range or something like that? If not I would rather just be for isDelta as I assume there cannot be more than 2 value, or can?
There was a problem hiding this comment.
range of data may or may not be meaningful. Results may or may not even have timestamps.
what do you mean two values? the DataFrame results can be any shape.
This PR proposes we add a request and response option to identify if that we want/got results as changes or full DataFrames.
There was a problem hiding this comment.
The StreamingResponseShape right now can be full or delta. Can there be third type?
There was a problem hiding this comment.
or do you mean use boolean rather than an enum? I think that makes sense... the name works well for the response (isDelta:true), but not sure about the request sendDelta? wantDelta?
There was a problem hiding this comment.
reqesting a delta of the data => request.isDelta kinda works for me but just a suggestion, do not know if that can be ambiguous in some context.
| export interface StramingQueryOptions { | ||
| frames?: StreamingResponseShape; | ||
| listener?: ObserverConfig; | ||
| notification?: ObserverConfig; |
There was a problem hiding this comment.
Honestly no idea how this should work. If I want a stream of data from datasource I call query function with DataQueryRequest as an argument. Does it make sense to send an option that says pause: true what will that do? If I want to pause it will I call query() function again or what?
There was a problem hiding this comment.
spitballing here :) and am also not sure how this should work.
It would be good to share semantics across panels and datasources. By adding options to the request, we could handle that in panelQueryState and/or DataSources directly.
In the non-trivial streaming datasources I have tried, they:
- get a request, check what data it wants
- if a stream is already open, add a subscription
- otherwise create a new server request and set up subscription
In this structure, a subsequent request to the same data could ask to pause the notifications, but the DataSource keeps collecting values.
There was a problem hiding this comment.
So do you mean DataSource would keep track of query -> stream and do some kind of deduplication? And that by giving the same query but with different options you could do actions on that query stream?
There was a problem hiding this comment.
something like that.
I am already checking if the same query exists to avoid setting up multiple web-sockets etc to the same data. (a non-issue in explore, a huge issue in panels)
There was a problem hiding this comment.
in the case of the loki query streams -- the url is the real unique identifier, not the refId. Anything pointing to the same url should share the same websocket connection
There was a problem hiding this comment.
I am again wondering if that is not too much to put into a DS. I mean if we had reusable query model and suitable UI to that this would not be a huge problem I think and would leave the DS implementation relatively naive and easy. Even if we say this is a requirement it is not something that can be guarantied by 3rd party datasources (definitely not by old ones) so I feel we would be implementing a deduplication layer anyway in our code.
There was a problem hiding this comment.
I'm torn on exactly where it should happen -- either in DS or panelQueryState, and maybe in super special cases in your custom panel.
- DataSource has direct access and can make the most efficient reasonable choices as long as our query semantics are clear. We can also create easy ways for DS to implement/extend standard patterns that are only known in the DS.
- panelQueryState: could encapsulate the default best guess implementation
- your custom panel: can do whatever it needs, but can not be easily reused
For now, I removed the pause options -- I added it really just to get us thinking about it for the future :)
* grafana/master: @grafana/data: improve the CircularVector api (grafana#18716)
|
@aocenas -- thanks for feedback. I updated the PR and removed the pause sketch. That was included just to get us thinking about what may be possible |
|
Closing this and merging with #18709 |
Looking at how to remove
deltaand settle on common streaming semantics... I think we should add some well-typed options to theDataQueryRequestthat support streaming.The immediate need to ask for
deltavsfullresults. However while looking at the work in #18696, it seems we should also have clear semantics on pause and throttle. In particular are we pausing all events (the listener) or do we want them back when we unpause (the notification)?Posting here to get quick feedback ideas etc. all names are looking for better alternatives :) If people like this, I will add it to #18709 and get it working there.