[bugfix] Correct end time adjustment for query range#6360
[bugfix] Correct end time adjustment for query range#6360ruslan-mikhailov merged 9 commits intografana:mainfrom
Conversation
ruslan-mikhailov
force-pushed
the
bugfix/recent-query-end-cutoff
branch
from
a21f517 to
8510258
Compare
ruslan-mikhailov
force-pushed
the
bugfix/recent-query-end-cutoff
branch
from
8510258 to
7a5290b
Compare
ruslan-mikhailov
requested review from
carles-grafana,
electron0zero,
ie-pham,
javiermolinar,
joe-elliott,
mapno,
mattdurham,
mdisibio,
oleg-kozlyuk-grafana,
stoewer,
yvrhdn and
zalegrala
as code owners
javiermolinar
left a comment
javiermolinar
left a comment
There was a problem hiding this comment.
Is there any e2e test we can test this?
| []pipeline.AsyncMiddleware[combiner.PipelineResponse]{ | ||
| headerStripWare, | ||
| adjustEndWareNanos, | ||
| // adjustEndWareNanos, // due to alignments and combiner, it needs to be done in handler |
There was a problem hiding this comment.
This is not just for clamping the end, it also provides a default start and end. The idea is to simplify the existing logic by relaying on it. Is there a way to keep it?
There was a problem hiding this comment.
Summary of the discussion: currently, it is not possible to do. In order to support the middleware is to force combiner initialisation only after all handlers are applied. There are a couple of ways to do it, but it's not a minor refactoring, so we decided to do it separately.
There was a problem hiding this comment.
Added ToDos
| maxEnd := now.Add(-cfg.QueryEndCutoff) | ||
| reqEnd := time.Unix(0, int64(req.End)) | ||
| if reqEnd.After(maxEnd) { | ||
| req.End = uint64(maxEnd.UnixNano()) |
There was a problem hiding this comment.
What's wrong with the existing reqEnd? The code does something similar, substracting the EndCutOff to time.Now()
3 participants
What this PR does: fixes
QueryEndCutofffor query range. Query frontend initialises query range handler with combiners and alignment first, which leads to two bugs: empty buckets after cutoff and ignoring the param if end is not aligned.Before:
After:
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]