-
Notifications
You must be signed in to change notification settings - Fork 11.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP/Idea: Streaming: add common request options #18728
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -377,6 +377,13 @@ export interface DataStreamState { | |
*/ | ||
delta?: DataFrame[]; | ||
|
||
/** | ||
* Is the data the entire buffer, or just the changes | ||
* | ||
* NOTE: optional now, but should be required | ||
*/ | ||
shape?: StreamingResponseShape; | ||
|
||
/** | ||
* Stop listening to this stream | ||
*/ | ||
|
@@ -437,6 +444,29 @@ export interface ScopedVars { | |
[key: string]: ScopedVar; | ||
} | ||
|
||
enum StreamingResponseShape { | ||
/** | ||
* Full DataFrames | ||
*/ | ||
full = 'full', | ||
|
||
/** | ||
* Send only the changes since the last notifiction | ||
*/ | ||
delta = 'delta', | ||
} | ||
|
||
interface ObserverConfig { | ||
pause?: boolean; | ||
throttle?: number; | ||
} | ||
|
||
export interface StramingQueryOptions { | ||
frames?: StreamingResponseShape; | ||
listener?: ObserverConfig; | ||
notification?: ObserverConfig; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly no idea how this should work. If I want a stream of data from datasource I call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 In the non-trivial streaming datasources I have tried, they:
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm torn on exactly where it should happen -- either in DS or
For now, I removed the pause options -- I added it really just to get us thinking about it for the future :) |
||
} | ||
|
||
export interface DataQueryRequest<TQuery extends DataQuery = DataQuery> { | ||
requestId: string; // Used to identify results and optionally cancel the request in backendSrv | ||
timezone: string; | ||
|
@@ -451,6 +481,7 @@ export interface DataQueryRequest<TQuery extends DataQuery = DataQuery> { | |
intervalMs: number; | ||
maxDataPoints: number; | ||
scopedVars: ScopedVars; | ||
stream?: StramingQueryOptions; | ||
|
||
// Request Timing | ||
startTime: number; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
StreamingResponseShape
right now can befull
ordelta
. Can there be third type?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.