Skip to content
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

Support with for window #13

Closed
wants to merge 3 commits into from

Conversation

lujiajing1126
Copy link

Resolve #12

Signed-off-by: Megrez Lu <lujiajing1126@gmail.com>
@lujiajing1126 lujiajing1126 marked this pull request as draft March 28, 2023 10:51
Signed-off-by: Megrez Lu <lujiajing1126@gmail.com>
Signed-off-by: Megrez Lu <lujiajing1126@gmail.com>
@lujiajing1126 lujiajing1126 marked this pull request as ready for review March 29, 2023 01:20
@lujiajing1126
Copy link
Author

@valyala @hagen1778 Request for view

@hagen1778 hagen1778 requested a review from valyala March 29, 2023 07:17
valyala added a commit that referenced this pull request Jul 19, 2023
…ions

Support using WITH template vars in the following places where duration can be passed:

- Lookbehind windows in square brackets: `with (w=5m) m[w]` is transformed to `m[5m]`
- Steps in square brackets: `with (step=5m) m[1h:step]` is transformed to `m[1h:5m]`
- Offsets: `with (off=5m) m offset off` is transformed to `m offset 5m`

Updates VictoriaMetrics/VictoriaMetrics#4025
Updates #12

Thanks to @lujiajing1126 for the initial implementation at #13

Note that this feature doesn't allow specifying dynamic durations in the following way:

  with (w = ((day_of_month()-1) * 24 + hour()) + "h") m[w]

It allows using only static durations

Support for dynamic durations requires significant refactoring of the code responsible for calculating rollups in `/api/v1/query_range` handler.
It will be needed to use different lookbehind windows per each calculated data point per each `step`. Currently the code assumes
that the lookbehind window is static across every calculated data point.
valyala added a commit to VictoriaMetrics/VictoriaMetrics that referenced this pull request Jul 19, 2023
…0.61.1

This adds support for passing durations via WITH template vars:

- `WITH (w = 5m) m[w]` is transformed to `m[5m]`
- `WITH (f(w, step, off) = m[w:step] offset off) f(5m, 10s, 1h)` is transformed to `m[5m:10s] offset 1h`

Updates #4025
Updates VictoriaMetrics/metricsql#12

See also the initial implementation by @lujiajing1126 at VictoriaMetrics/metricsql#13
valyala added a commit to VictoriaMetrics/VictoriaMetrics that referenced this pull request Jul 19, 2023
…0.61.1

This adds support for passing durations via WITH template vars:

- `WITH (w = 5m) m[w]` is transformed to `m[5m]`
- `WITH (f(w, step, off) = m[w:step] offset off) f(5m, 10s, 1h)` is transformed to `m[5m:10s] offset 1h`

Updates #4025
Updates VictoriaMetrics/metricsql#12

See also the initial implementation by @lujiajing1126 at VictoriaMetrics/metricsql#13
@valyala
Copy link
Contributor

valyala commented Jul 19, 2023

@lujiajing1126 , thanks for the pull request! I used it as a basis for the commit f3dd382 .
This commit adds support for passing duration args via WITH templates. For example:

  • WITH (w = 5m) m[w] is expanded into m[5m]
  • WITH (f(w, step, off) = m[w:step] offset off) f(5m, 10s, 1h) is expanded into m[5m:10s] offset 1h

Unfortunately the commit doesn't allow passing dynamically calculated duration values depending on the evaluation timestamp, because this requires significant refactoring in the code responsible for calculating rollup function results per each returned data point from /api/v1/query_range. The temporary workaround is to calculate the needed duration values at client side and pass them to /api/v1/query depending on the time query arg.

As for the original issue VictoriaMetrics/VictoriaMetrics#4025 , we need to think more how to properly implement it properly. The m[((day_of_month()-1) * 24 + hour()) + "h"] approach looks too complex to me. Probably, it would be great to introduce new duration aliases such as start_of_hour, start_of_day, start_of_week, etc, which could be used inside MetricsQL queries. For example, increase(http_requests_total[start_of_day]) would return the number of requests for the current day by UTC. The syntax should support specifying timezone offset, obviously. Something like increase(http_requests_total[start_of_day timezone "+03:00"]) could return the number of requests for the current day at timezone +03:00.

Closing the pull request then, since it has been overridden by the commit f3dd382 .

@valyala
Copy link
Contributor

valyala commented Jul 28, 2023

FYI, the functionality described in this comment has been included in VictoriaMetrics v1.92.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support seting dynamic time duration
2 participants