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.

Sign up for GitHub

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

Jump to bottom

Promtail: support unix timestamps with fractional seconds #2301

Merged
merged 5 commits into from
Jul 6, 2020
Merged

Promtail: support unix timestamps with fractional seconds #2301

merged 5 commits into from
Jul 6, 2020

Conversation

flixr
Copy link
Contributor

@flixr flixr commented Jul 6, 2020 •
edited

If the timestamp contains a ., parse the fractions into nanoseconds.

What this PR does / why we need it:
See #2193

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

Special notes for your reviewer:
There might be a nicer way, but I'm a Go newbie...

Checklist

  • Documentation added
  • Tests updated

@CLAassistant
Copy link

CLAassistant commented Jul 6, 2020 •
edited

CLA assistant check
All committers have signed the CLA.

cyriltovena
cyriltovena reviewed Jul 6, 2020
View reviewed changes
Copy link
Contributor

@cyriltovena cyriltovena left a comment •
edited

Choose a reason for hiding this comment

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

Awesome thank you for this.

Could you add some tests case ? we have a test function in util_test.go named TestConvertDateLayout

@pull-request-size pull-request-size bot added size/M and removed size/S labels Jul 6, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jul 6, 2020 •
edited

Codecov Report

Merging #2301 into master will decrease coverage by 0.01%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2301      +/-   ##
==========================================
- Coverage   62.27%   62.25%   -0.02%     
==========================================
  Files         158      158              
  Lines       12766    12776      +10     
==========================================
+ Hits         7950     7954       +4     
- Misses       4201     4205       +4     
- Partials      615      617       +2
Impacted Files Coverage Δ
pkg/logentry/stages/util.go 57.62% <60.00%> (+0.21%) ⬆️
pkg/querier/queryrange/downstreamer.go 95.87% <0.00%> (-2.07%) ⬇️

cyriltovena
cyriltovena approved these changes Jul 6, 2020
View reviewed changes
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 nit and we're good to merge.

@flixr @cyriltovena
don't capitalize error
f65e65e 
Co-authored-by: Cyril Tovena <cyril.tovena@gmail.com>
@cyriltovena
Copy link
Contributor

Again thank you so much @flixr for this contribution, I'll merge it as soon as it builds.

@flixr
Copy link
Contributor Author

flixr commented Jul 6, 2020

my pleasure, thanks for the quick review!

@cyriltovena
Copy link
Contributor

I'm looking at that CI issue, not your fault btw.

@cyriltovena cyriltovena merged commit 616771a into grafana:master Jul 6, 2020
@flixr flixr deleted the unix_timestamp_with_fractions branch July 6, 2020 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyriltovena cyriltovena cyriltovena approved these changes

Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

promtail: support unix timestamps with fractional seconds
4 participants
@flixr @CLAassistant @codecov-commenter @cyriltovena