-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Plugins parsers csv unix timestamp #5047
Plugins parsers csv unix timestamp #5047
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! While this works, I think it would be nice to eventually centralize the json time format logic into the internal
package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tkanos, in order to be closer in behavior to the other parsers can you make it so that if the csv_timestamp_format = "unix"
then the format is parsed as unix time.
If we try to use unix timestamp in the csv file, we will get an error. This commit fixes that. * if the csv_timestamp_format = "unix" we try to parse the timestamp assuming that we will get an epoch in ms. * Put all the parsing timestamp part in a function (as it deserves it :D)
5007b2a
to
f2fdaae
Compare
Thanks @danielnelson for the advice. It was done. |
@@ -75,7 +75,8 @@ document. | |||
|
|||
The `csv_timestamp_column` option specifies the column name containing the | |||
time value and `csv_timestamp_format` must be set to a Go "reference time" | |||
which is defined to be the specific time: `Mon Jan 2 15:04:05 MST 2006`. | |||
which is defined to be the specific time: `Mon Jan 2 15:04:05 MST 2006`, | |||
it can also be `unix` (for epoch in ms format like 1257894000 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this for seconds, not ms, so worries I'll switch it post merge.
Required for all PRs:
Description.
For now the csv parser does not handle unix timestamp like
1257894000
as csv_timestamp_format in the csv as timestamp.So to fix that if csv_timestamp_format = Unix, we assume that the timestamp is an epoch ms format.
I have also added unit tests, and no others unit test were broken.
=== RUN TestBasicCSV
--- PASS: TestBasicCSV (0.00s)
=== RUN TestHeaderConcatenationCSV
--- PASS: TestHeaderConcatenationCSV (0.00s)
=== RUN TestHeaderOverride
--- PASS: TestHeaderOverride (0.00s)
=== RUN TestTimestamp
--- PASS: TestTimestamp (0.00s)
=== RUN TestTimestampError
--- PASS: TestTimestampError (0.00s)
=== RUN TestTimestampUnixFormat
--- PASS: TestTimestampUnixFormat (0.00s)
=== RUN TestQuotedCharacter
--- PASS: TestQuotedCharacter (0.00s)
=== RUN TestDelimiter
--- PASS: TestDelimiter (0.00s)
=== RUN TestValueConversion
--- PASS: TestValueConversion (0.00s)
=== RUN TestSkipComment
--- PASS: TestSkipComment (0.00s)
=== RUN TestTrimSpace
--- PASS: TestTrimSpace (0.00s)
=== RUN TestSkipRows
--- PASS: TestSkipRows (0.00s)
=== RUN TestSkipColumns
--- PASS: TestSkipColumns (0.00s)
=== RUN TestSkipColumnsWithHeader
--- PASS: TestSkipColumnsWithHeader (0.00s)
=== RUN TestParseStream
--- PASS: TestParseStream (0.00s)
PASS
ok github.com/influxdata/telegraf/plugins/parsers/csv