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

Add fallback_formats for timestamp stage #2615

Merged

Conversation

aminjam
Copy link
Contributor

@aminjam aminjam commented Sep 10, 2020

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

Checklist

  • Documentation added
  • Tests updated

@CLAassistant
Copy link

CLAassistant commented Sep 10, 2020

CLA assistant check
All committers have signed the CLA.

@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2020

Codecov Report

Merging #2615 into master will decrease coverage by 1.60%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2615      +/-   ##
==========================================
- Coverage   63.13%   61.52%   -1.61%     
==========================================
  Files         169      169              
  Lines       15018    13154    -1864     
==========================================
- Hits         9481     8093    -1388     
+ Misses       4784     4322     -462     
+ Partials      753      739      -14     
Impacted Files Coverage Δ
pkg/logentry/stages/timestamp.go 86.41% <66.66%> (-3.46%) ⬇️
pkg/promtail/positions/positions.go 46.80% <0.00%> (-12.85%) ⬇️
pkg/promtail/targets/file/tailer.go 68.53% <0.00%> (-10.63%) ⬇️
pkg/logql/step_evaluator.go 57.14% <0.00%> (-9.53%) ⬇️
pkg/logql/marshal/labels.go 66.66% <0.00%> (-8.34%) ⬇️
pkg/logproto/timestamp.go 40.00% <0.00%> (-6.81%) ⬇️
pkg/chunkenc/encoding_helpers.go 59.25% <0.00%> (-6.37%) ⬇️
pkg/logql/marshal/tail.go 66.66% <0.00%> (-6.07%) ⬇️
pkg/promtail/api/types.go 14.28% <0.00%> (-5.72%) ⬇️
pkg/logql/vector.go 81.81% <0.00%> (-5.69%) ⬇️
... and 160 more

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

@slim-bean if you're ok with the name of the property I let you merge it.

@cyriltovena
Copy link
Contributor

@aminjam in the meantime can you sign the CLA ?

Copy link
Contributor

@sandeepsukhani sandeepsukhani left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the PR!

@slim-bean
Copy link
Collaborator

Hi @aminjam would love to merge this but we need to get the CLA "signed".

If you've "signed" it and it fails, this is usually because the email address in your commit doesn't match any in your github account.

Fixing this usually requires amending the commit with an email address you have in github and force pushing back to your branch.

Let me know if you have any questions or problems!

@aminjam
Copy link
Contributor Author

aminjam commented Sep 21, 2020

Thank you all for your patience. I have been working through getting the authorization to sign the CLA. I hope to have more updates on the progress this week.

@winkingturtle-vmw winkingturtle-vmw force-pushed the timestamp-stage-with-fallback-formats branch from 590e795 to d05a4ab Compare September 25, 2020 22:32
@slim-bean
Copy link
Collaborator

Nice thanks again @aminjam!

@slim-bean slim-bean merged commit 9fcb8b8 into grafana:master Sep 28, 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.

timestamp stage with multiple formats
7 participants