-
Notifications
You must be signed in to change notification settings - Fork 460
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
prometheus codec time param parsing docstrings and emulating upstream prometheus behavior #6985
prometheus codec time param parsing docstrings and emulating upstream prometheus behavior #6985
Conversation
… prometheus behavior
@@ -255,6 +255,8 @@ func (c prometheusCodec) decodeInstantQueryRequest(r *http.Request) (Request, er | |||
return &result, nil | |||
} | |||
|
|||
// DecodeRangeQueryTimeParams encapsulates Prometheus instant query time param parsing, | |||
// emulating the logic in prometheus/prometheus/web/api/v1#API.query_range. |
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.
I don't feel particularly strongly about this, but what do you think about a direct link?
// emulating the logic in prometheus/prometheus/web/api/v1#API.query_range. | |
// emulating the logic in https://github.com/prometheus/prometheus/blob/0763ec841bdc75d163a123dbffd05b12ff0500f1/web/api/v1/api.go#L505-L582. |
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.
(same for the other ones)
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.
neat, we can probably remove this piece of code after merging your change. It's simpler to think about this as part of the decoding logic more than as part of the generic request handling logic
mimir/pkg/frontend/querymiddleware/roundtrip.go
Lines 422 to 439 in e14de95
func defaultInstantQueryParamsRoundTripper(next http.RoundTripper) http.RoundTripper { | |
return RoundTripFunc(func(r *http.Request) (*http.Response, error) { | |
if IsInstantQuery(r.URL.Path) && !r.Form.Has("time") && !r.URL.Query().Has("time") { | |
nowUnixStr := strconv.FormatInt(time.Now().Unix(), 10) | |
q := r.URL.Query() | |
q.Add("time", nowUnixStr) | |
r.URL.RawQuery = q.Encode() | |
// If form was already parsed, add this param to the form too. | |
// (The form doesn't have "time", otherwise we'd not be here) | |
if r.Form != nil { | |
r.Form.Set("time", nowUnixStr) | |
} | |
} | |
return next.RoundTrip(r) | |
}) | |
} |
I opened #7026 to do the clean up |
re-reviewed these utils against the upstream prometheus implementation.
No behavior changes, just docstrings referencing the canonical implementation, and updating the signature of the instant query decoder to take a
time.Now
func like upstream does.What this PR does
Which issue(s) this PR fixes or relates to
Fixes #
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]
.about-versioning.md
updated with experimental features.