Pass query hints down into remote read query proto#4122
Pass query hints down into remote read query proto#4122brian-brazil merged 2 commits intoprometheus:masterfrom henridf:queryparams-remoteread
Conversation
Signed-off-by: Henri DF <[email protected]>
prompb/remote.proto
Outdated
| int64 start_timestamp_ms = 1; | ||
| int64 end_timestamp_ms = 2; | ||
| repeated prometheus.LabelMatcher matchers = 3; | ||
| prometheus.ReadParams params = 4; |
There was a problem hiding this comment.
Hints would be clearer than params I think
There was a problem hiding this comment.
I modeled this on SelectParams, but I agree that Hints is clearer; changed.
(we should maybe also rename SelectParams -> SelectHints)
prompb/types.proto
Outdated
| } | ||
|
|
||
| message ReadParams { | ||
| int64 step = 1; |
There was a problem hiding this comment.
You should mention the unit here
Signed-off-by: Henri DF <[email protected]>
|
|
||
| message ReadHints { | ||
| int64 step_ms = 1; // Query step size in milliseconds. | ||
| string func = 2; // String representation of surrounding function or aggregation. |
There was a problem hiding this comment.
Could it be any function? Or we should limit it to some small set common for remote storages? Maybe enum will be better?
There was a problem hiding this comment.
It's only a hint, and it can be any function.
|
I looked into Prometheus code, and it is not clear to me if those hints can be ignored by remote storage. Can they? |
|
@brian-brazil i addressed your comments, anything further needed to merge this PR? |
|
Thanks! |
Signed-off-by: Henri DF <[email protected]>
4 participants
Completes #2580
This currently doesn't include tests, but I'd gladly add some if there are suggestions on how/where to do this.