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

Loki: refactor validation and improve error messages #2021

Merged
merged 5 commits into from
Apr 30, 2020

Conversation

slim-bean
Copy link
Collaborator

@slim-bean slim-bean commented Apr 30, 2020

this copies some validation code from Cortex into Loki modifies it to be more appropriate for Loki, this gives us a single metric we can use for detecting validation errors (previously we had to check cortex_ and loki_ metrics) as well as lets us use error messages more appropriate for logs than metrics.

Signed-off-by: Ed Welch edward.welch@grafana.com

Checklist

  • Documentation added
  • Tests updated

…propriate for Loki, this gives us a single metric we can use for detetecting validation errors (previously we had to check cortex_ and loki_ metrics) as well as lets us use error messages more appropriate for logs than metrics.

Signed-off-by: Ed Welch <edward.welch@grafana.com>
Signed-off-by: Ed Welch <edward.welch@grafana.com>
Copy link
Member

@owen-d owen-d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I'm excited to get some better validations merged.

It looks good, but I'd like to see some of the error messages exported as functions instead. That will allow the compiler to check function arity and help ensure we don't have argument/error template drift in the future.

pkg/distributor/distributor.go Show resolved Hide resolved
pkg/distributor/distributor.go Outdated Show resolved Hide resolved
pkg/distributor/validator.go Outdated Show resolved Hide resolved
pkg/distributor/validator.go Outdated Show resolved Hide resolved
pkg/distributor/validator.go Outdated Show resolved Hide resolved
pkg/distributor/validator.go Outdated Show resolved Hide resolved
pkg/distributor/validator.go Outdated Show resolved Hide resolved
Signed-off-by: Ed Welch <edward.welch@grafana.com>
Signed-off-by: Ed Welch <edward.welch@grafana.com>
Signed-off-by: Ed Welch <edward.welch@grafana.com>
@slim-bean slim-bean merged commit 3dfe893 into master Apr 30, 2020
@slim-bean slim-bean deleted the refactor-validation branch April 30, 2020 19:43
slim-bean added a commit that referenced this pull request Apr 30, 2020
* copies some of the validation logic from cortex and modifies it to appropriate for Loki, this gives us a single metric we can use for detetecting validation errors (previously we had to check cortex_ and loki_ metrics) as well as lets us use error messages more appropriate for logs than metrics.

Signed-off-by: Ed Welch <edward.welch@grafana.com>

* fixing tests and lint

Signed-off-by: Ed Welch <edward.welch@grafana.com>

* updating cause to string type

Signed-off-by: Ed Welch <edward.welch@grafana.com>

* extracting a function for rate limited

Signed-off-by: Ed Welch <edward.welch@grafana.com>

* extracting functions for the rest of the error strings

Signed-off-by: Ed Welch <edward.welch@grafana.com>
(cherry picked from commit 3dfe893)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants