Query Frontend: Expose samples_processed in Server-Timing header #10103
Conversation
There was a problem hiding this comment.
I have a comment about the naming. "total samples" may mean "total raw samples processed" or "total number of samples returned by the query". The cost of a query is a function of the actual raw samples processed, not returned. If that's what you're going to track (as I would expect) then we may consider renaming it to "samples processed" (to keep it consistent with "bytes processed" that we already have).
tinitiuset
force-pushed
the
tinitiuset/feat/exposeTotalSamples
branch
from
c373bdd to
cc1117d
Compare
tinitiuset
changed the title
|
@pracucci 's feedback is acknowledged and MQE support will be added in another PR. |
|
The CHANGELOG has just been cut to prepare for the next release. Please rebase |
|
Ready for review again. Testing updated. CHANGELOG updated also. |
tinitiuset
force-pushed
the
tinitiuset/feat/exposeTotalSamples
branch
from
560ff96 to
a39869b
Compare
…s affecting other tests and Mimir doesn't support Prometheus' UTF-8 metric/label name scheme yet.
tinitiuset
force-pushed
the
tinitiuset/feat/exposeTotalSamples
branch
from
a39869b to
2969331
Compare
|
Branch got out of sync with main. I also updated a test that was making other tests fail because it was enabling UTF-8 support. |
| }, | ||
| "sparse_series": { | ||
| expr: `sparse_series{}`, | ||
| expectedTotalSamples: 5 + 4, |
There was a problem hiding this comment.
shouldn't this be 2 because there are only two samples
sparse_series 0 _ _ _ _ _ _ 7 _ _ _
There was a problem hiding this comment.
I'm not completely sure why, but in that case, the engine is returning [0, "0"], [60, "0"], [120, "0"], [180, "0"], [240, "0"], [420, "7"], [480, "7"], [540, "7"], [600, "7"] which equal to 9 samples. I also assumed 2 samples beforehand.
There was a problem hiding this comment.
i wonder if it's not because of the lookback period of 5 minutes. Can you quickly verify that it is? It's configurable on the promQL engine config via LookbackDelta
There was a problem hiding this comment.
Good call, I tried different LookbackDelta values and it's consistent with the samples returned.
What this PR does
This PR will make the Query frontend expose samples_processed in Server-Timing header. It is complementary to #9985 and this in the effort of measuring throughput on Mimir.
This was worked on by @krajorama before #7966, but I decided to open a different PR as the idea has evolved overtime.
Which issue(s) this PR fixes or relates to
Not related to an specific issue.
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.