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

Clearer error response from push endpoint when labels are malformed #1750

Merged
merged 2 commits into from
Mar 2, 2020

Conversation

talham7391
Copy link

What this PR does / why we need it:
Currently, when users use the /loki/api/v1/push with a malformed labels, they get an error response saying something along the lines of:
parse error at line 1, col 4: literal not terminated.

It isn't clear that the labels are the issue. This PR makes it clear.

Which issue(s) this PR fixes:
Fixes #1727

Checklist

  • Documentation added
  • Tests updated

@claassistantio
Copy link

claassistantio commented Feb 27, 2020

CLA assistant check
All committers have signed the CLA.

@talham7391
Copy link
Author

talham7391 commented Feb 27, 2020

I signed the agreement, made sure my emails are added to my account as well. Not sure why it says I haven't signed it 🤔

@slim-bean
Copy link
Collaborator

Hi @talham7391 thanks for the PR!

It looks like the email associated with the commit is not valid and therefore github can't link it to your account.

You would need to git config --global user.email "email@example.com" with an email associated to your github account and re-write the commit/force push back to the PR I think for the CLA to be happy.

@talham7391
Copy link
Author

Thanks! @slim-bean

@codecov-io
Copy link

codecov-io commented Feb 28, 2020

Codecov Report

Merging #1750 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1750      +/-   ##
==========================================
- Coverage   64.33%   64.28%   -0.05%     
==========================================
  Files         121      121              
  Lines        9081     9081              
==========================================
- Hits         5842     5838       -4     
- Misses       2835     2837       +2     
- Partials      404      406       +2
Impacted Files Coverage Δ
pkg/distributor/validator.go 82.6% <100%> (ø) ⬆️
pkg/ingester/transfer.go 65% <0%> (-1.43%) ⬇️
pkg/logql/evaluator.go 91.22% <0%> (-0.59%) ⬇️

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM just a small suggestion if you have time.

@slim-bean slim-bean merged commit fa8bf2a into grafana:master Mar 2, 2020
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.

/api/prom/push produce error 400 with 'unexpected IDENTIFIER'
5 participants