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

configurable sub-second precision with no time key #249

Merged
merged 1 commit into from
May 5, 2017
Merged

configurable sub-second precision with no time key #249

merged 1 commit into from
May 5, 2017

Conversation

sampointer
Copy link

@sampointer sampointer commented Mar 28, 2017

For input plugins that do not provide a time key as part of the
record but do provide a time to the router, allow the degree
of sub-second time precision to be configurable.

Some sources (such as AWS CloudWatch) may provide an accurate time
source but do not include a time portion for an otherwise free-form
record: there is nothing to parse.

In this case the casting of a DateTime to a String loses any
sub-second precision.

(check all that apply)

  • tests added
  • tests passing
  • README updated (if needed)
  • README Table of Contents updated (if needed)
  • History.md and version in gemspec are untouched
  • backward compatible
  • feature works in elasticsearch_dynamic (not required but recommended)

@coveralls
Copy link

coveralls commented Mar 28, 2017

Coverage Status

Coverage increased (+0.01%) to 94.503% when pulling 64c7d7b on sampointer:add_configurable_time_precision_when_timestamp_missing into 83f978f on uken:master.

For input plugins that do not provide a time key as part of the
record but do provide a `time` to the router, allow the degree
of sub-second time precision to be configurable.

Some sources (such as AWS CloudWatch) may provide an accurate time
source but do not include a time portion for an otherwise free-form
record: there is nothing to parse.

In this case the casting of a `DateTime` to a `String` loses any
sub-second precision.
@coveralls
Copy link

coveralls commented Mar 28, 2017

Coverage Status

Coverage increased (+0.01%) to 94.503% when pulling e3650a7 on sampointer:add_configurable_time_precision_when_timestamp_missing into 83f978f on uken:master.

@sampointer
Copy link
Author

Hi! Is there anything I can do to help expedite this PR?

@Tom-Fawcett
Copy link

@sampointer how do you think this compares to #223?

@sampointer
Copy link
Author

sampointer commented May 4, 2017

They're solving two different problems.

#223 is basically doing "fluentd API change stuff" for plugins, which basically amounts to moving some stuff around in the class hierarchy. fluentd have stuffed semver for their plugin API: fluent/fluentd#1449

This fixes a specific bug (and it is a bug) with a type cast losing time granularity in a specific situation.

They should both be merged.

@Tom-Fawcett
Copy link

@sampointer thanks for your explanation, I was just curious due to a specific similarity between your pull and #223 -

#223

+          record[TIMESTAMP_FIELD] = dt.iso8601(9).to_s

#249

+          record[TIMESTAMP_FIELD] = dt.iso8601(@time_precision)

Anyway, regarding getting this merged; @gihad 's comment on #250 explains that there is unfortunately no active maintainer at this time.

@gihad
Copy link
Contributor

gihad commented May 5, 2017

@sampointer I took a look at this PR - seems backwards compatible and everything looks good to me. Thanks for the contribution.

Regarding #223, does it looks good to you? I haven't looked in depth yet and I noticed there might be an outstanding question.

I will merge this one.

@gihad gihad merged commit d5ad749 into uken:master May 5, 2017
@Tom-Fawcett
Copy link

Tom-Fawcett commented May 5, 2017

@gihad thanks for merging this one.

I notice that https://rubygems.org/gems/fluent-plugin-elasticsearch is still only showing 1.9.3 and not the two newer versions. Is that something you can resolve?

EDIT - @pitr helped out and published the newer versions.

sampointer pushed a commit to ConnectedHomes/omnibus-td-agent that referenced this pull request May 23, 2017
This will fix:

- fluent-binlog-reader
  (fluent/fluentd@155ceac)
- ms timestamp resolution for cloudwatch
  (uken/fluent-plugin-elasticsearch#249)
@sampointer sampointer deleted the add_configurable_time_precision_when_timestamp_missing branch September 28, 2020 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants