filebeat @timestamp support nanoseconds #7576

ghost · 2018-07-12T08:20:49Z

Hi! I am not an original author of this patch, though can contribute.

This PR

adds nanosecond support to dtfmt module
passes nanoseconds in @timestamp to downstream consumers

dtfmt is basically created similar to DateTimeFormat from the Joda Time. It does not support nanosecond precision, though Java itself can now parse nanoseconds which was introduced with JSR-310. The format is slightly different, with this PR you should use n for fractional digits instead of S, in Java you can just append as much S as you want. We've tested this and it seems to work, now we need more thorough review from experienced contributors. I'm not familiar with Go programming but can spent some time to make fixes if needed.

Thanks!

Related GitHub issue: #7559
Discuss topic: https://discuss.elastic.co/t/low-timestamp-precision-cause-wrong-order-of-events-with-docker-input/137051/4

elasticmachine · 2018-07-12T08:20:50Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

houndci-bot · 2018-07-12T08:21:24Z

libbeat/outputs/codec/common.go

+	}
+}
+
+func MakeBCNanoTimestampEncoder() func(*common.Time, structform.ExtVisitor) error {


exported function MakeBCNanoTimestampEncoder should have comment or be unexported

houndci-bot · 2018-07-12T08:21:24Z

libbeat/outputs/codec/common.go

@@ -33,3 +33,29 @@ func MakeBCTimestampEncoder() func(*common.Time, structform.ExtVisitor) error {
 		return enc((*time.Time)(t), v)
 	}
 }
+
+func MakeNanoTimestampEncoder() func(*time.Time, structform.ExtVisitor) error {


exported function MakeNanoTimestampEncoder should have comment or be unexported

ruflin · 2018-07-13T08:12:13Z

Thanks for the contribution. We also had an internal discussion on if this should be turned on by default for all future release or not. We came to the conclusion that this is not a breaking change and we can have it on by default, no config option needed.

@urso Could you have a look?

urso · 2018-07-31T16:47:31Z

libbeat/outputs/codec/json/json.go

@@ -58,6 +58,8 @@ func (e *Encoder) reset() {
 		gotype.Folders(
 			codec.MakeTimestampEncoder(),
 			codec.MakeBCTimestampEncoder(),
+			codec.MakeNanoTimestampEncoder(),
+			codec.MakeBCNanoTimestampEncoder(),


choose one, but not both. MakeTimestampEncoder and MakeNanoTimestampEncoder have the same function signature. So to not confuse setup, select only one. Note to myself: add panic if encoders with same signature are passed.

@urso you mean that remove codec.MakeTimestampEncoder() and codec.MakeBCTimestampEncoder() function from gotype.Folders, is it?

Note to myself: add panic if encoders with same signature are passed.

I don't know what mean. MakeNanoTimestampEncoder has add panic, you can check https://github.com/elastic/beats/pull/7576/files/8996d4f0e27415eb7d390ccc0da694bad76969bf#diff-1983a46731b9d0b0b4bc88ec99b97794R41. Or other changes?

Yes, remove them.

The encoder uses reflection to choose the right function depending on the function paramters and actual type. One type must have only one function registered. e.g. MakeTimestampEncoder and MakeNanoTimestampEncoder have the same type func(*time.Time, structform.ExtVisitor) error. Which one shall the encoder use?

MakeNanoTimestampEncoder has add panic, you can check /pull/7576/files/8996d4f0e27415eb7d390ccc0da694bad76969bf#diff-1983a46731b9d0b0b4bc88ec99b97794R41. Or other changes?

Stack-trace with content of actual error message would be helpful. I guess you mean the panic in the error right after dtfmt.NewFormatter?

urso · 2018-07-31T16:47:51Z

libbeat/outputs/elasticsearch/enc.go

-			codec.MakeBCTimestampEncoder()))
+			codec.MakeBCTimestampEncoder(),
+			codec.MakeNanoTimestampEncoder(),
+			codec.MakeBCNanoTimestampEncoder()))


Same here, choose one.

urso

+1 on adding nanosecond support. Please add tests to dfmt_test.go as well.

pytimer · 2018-12-25T03:51:36Z

@ruflin @urso Very sorry, i have some thing during this time.

I will fix this PR according to @urso feedback.

ruflin · 2018-12-27T09:09:20Z

@pytimer Great to hear. Also make sure to rebase on master as lots of things have change in the code base since then.

pytimer · 2018-12-28T03:44:51Z

Hi @ruflin

I encountered a question that i send a new commit, but it can not auto related to this PR.

My commit is pytimer@605733d

What should i do, Can you tell me? Thanks.

ruflin · 2018-12-28T08:43:18Z

@pytimer As it is not your PR you can't push to it. I recommend you to open your own PR and from there mention this PR that it's a follow up / take over of the PR.

pytimer · 2018-12-28T09:43:07Z

@ruflin Thanks you reponse.

I open a new PR #9818 about it, and i hope you can check it. Thanks.

urso · 2019-01-02T15:57:03Z

Can we close this PR?

botelastic · 2020-07-08T18:17:23Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

botelastic · 2020-07-08T18:17:41Z

This pull request doesn't have a Team:<team> label.

filebeat @timestamp support nanoseconds

8996d4f

houndci-bot reviewed Jul 12, 2018

View reviewed changes

ghost mentioned this pull request Jul 12, 2018

[Filebeat] docker input support nanoseconds timestamp #7559

Closed

ruflin requested a review from urso July 13, 2018 08:10

ruflin added enhancement review libbeat labels Jul 13, 2018

urso reviewed Jul 31, 2018

View reviewed changes

urso suggested changes Jul 31, 2018

View reviewed changes

pytimer mentioned this pull request Dec 28, 2018

[WIP]beats @timestamp support nanoseconds #9818

Merged

botelastic bot added Stalled needs_team Indicates that the issue/PR needs a Team:* label labels Jul 8, 2020

urso closed this Jul 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

filebeat @timestamp support nanoseconds #7576

filebeat @timestamp support nanoseconds #7576

ghost commented Jul 12, 2018 •

edited by ghost

Loading

elasticmachine commented Jul 12, 2018

houndci-bot Jul 12, 2018

houndci-bot Jul 12, 2018

ruflin commented Jul 13, 2018

urso Jul 31, 2018

pytimer Dec 25, 2018

urso Jan 2, 2019

urso Jul 31, 2018

urso left a comment

pytimer commented Dec 25, 2018

ruflin commented Dec 27, 2018

pytimer commented Dec 28, 2018

ruflin commented Dec 28, 2018

pytimer commented Dec 28, 2018

urso commented Jan 2, 2019

botelastic bot commented Jul 8, 2020

botelastic bot commented Jul 8, 2020

filebeat @timestamp support nanoseconds #7576

filebeat @timestamp support nanoseconds #7576

Conversation

ghost commented Jul 12, 2018 • edited by ghost Loading

elasticmachine commented Jul 12, 2018

houndci-bot Jul 12, 2018

Choose a reason for hiding this comment

houndci-bot Jul 12, 2018

Choose a reason for hiding this comment

ruflin commented Jul 13, 2018

urso Jul 31, 2018

Choose a reason for hiding this comment

pytimer Dec 25, 2018

Choose a reason for hiding this comment

urso Jan 2, 2019

Choose a reason for hiding this comment

urso Jul 31, 2018

Choose a reason for hiding this comment

urso left a comment

Choose a reason for hiding this comment

pytimer commented Dec 25, 2018

ruflin commented Dec 27, 2018

pytimer commented Dec 28, 2018

ruflin commented Dec 28, 2018

pytimer commented Dec 28, 2018

urso commented Jan 2, 2019

botelastic bot commented Jul 8, 2020

botelastic bot commented Jul 8, 2020

ghost commented Jul 12, 2018 •

edited by ghost

Loading