-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
feat: Add step param to Patterns Query API #12703
Conversation
{Timestamp: 1, Value: 2}, | ||
{Timestamp: 3, Value: 4}, | ||
{Timestamp: 5, Value: 6}, | ||
{Timestamp: 2, Value: 2}, |
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.
The stepping functionality shunts values into the step bucket before the sample TS, so these tests all had to be skewed down by to an even numbered millisecond as the step size is 2ms. Let me know if you don't agree with this behaviour.
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.
Sounds good, but you could have made the step configurable in the tests and default to previous behaviour.
eba1751
to
a0b6949
Compare
I think there updates requires in those files too. https://github.com/grafana/loki/blob/main/pkg/loghttp/patterns.go#L9 The frontend in distributed mode is a huge minefields, I suggest you give a go and test it in dev to make sure it works correctly. Hit me up if you need help for doing so. |
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.
Lgtm
The http parsing could use some tests but nothing blocking
What this PR does / why we need it:
step
parameter to the Patterns Query APIWhich issue(s) this PR fixes:
Fixes #12689
Special notes for your reviewer:
Checklist
CONTRIBUTING.md
guide (required)docs/sources/setup/upgrade/_index.md
production/helm/loki/Chart.yaml
and updateproduction/helm/loki/CHANGELOG.md
andproduction/helm/loki/README.md
. Example PRdeprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR