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

d (day) and w (week) argument doesn't work with offset modifier #5199

Closed
chinmaygupta28 opened this issue Jan 21, 2022 · 6 comments · Fixed by #5275
Closed

d (day) and w (week) argument doesn't work with offset modifier #5199

chinmaygupta28 opened this issue Jan 21, 2022 · 6 comments · Fixed by #5275
Assignees
Labels
component/logql good first issue These are great first issues. If you are looking for a place to start, start here! type/bug Somehing is not working as expected

Comments

@chinmaygupta28
Copy link

chinmaygupta28 commented Jan 21, 2022

Describe the bug
Only hour (h), minute (m), milliseconds (ms) and second (s) duration format works with the offset modifier but day (d), week (w) and year (y) throws this error = parse error at line 1, col 47: syntax error: unexpected NUMBER, expecting DURATION

To Reproduce
Make the 1d and 1w query visible to reproduce the error.

play.grafana.com link

Expected behavior
I expect them to work as they work in promql.

Environment:
Any

Screenshots, Promtail config, or terminal output

image
image

@dannykopping
Copy link
Contributor

@chinmaygupta28 thank you for reporting this.

This is indeed a bug. We state explicitly that we support time durations the same way Prometheus does, so this is an inconsistency.

@dannykopping dannykopping added component/logql good first issue These are great first issues. If you are looking for a place to start, start here! kind/bug labels Jan 21, 2022
@SasSwart
Copy link
Contributor

I'll look into this and prepare a Pull Request.

@chinmaygupta28
Copy link
Author

chinmaygupta28 commented Jan 21, 2022

@chinmaygupta28 thank you for reporting this.

This is indeed a bug. We state explicitly that we support time durations the same way Prometheus does, so this is an inconsistency.

Went through the docs and realized, year (y) doesn't work either, only minutes (m), hours (h) and milliseconds (ms) work. I will update the issue description.


Updated the main issue

@SasSwart
Copy link
Contributor

SasSwart commented Jan 24, 2022

Progress Update:

I've found the issue, but I'm still preparing the Pull Request.
Currently updating tests and ensuring that my changes don't have unintended side effects.

@SasSwart
Copy link
Contributor

Progress Update:

I've implemented a naive solution, but its not optimal. I'd like to explore an alternative, less complex, solution before submitting a PR. After an examination tomorrow afternoon, I'll continue work on this issue.

@SasSwart
Copy link
Contributor

Progress Update:

A draft PR is available
This is still actively WIP, so don't review yet, please.

@chaudum chaudum added the type/bug Somehing is not working as expected label Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/logql good first issue These are great first issues. If you are looking for a place to start, start here! type/bug Somehing is not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants