Use Time.at(time) instead of Time.now for fallback

[dannyk81]

I'm curious why not use Time.at(time) in the fallback option instead of Time.now, here

fluent-plugin-elasticsearch-timestamp-check/lib/fluent/plugin/filter_elasticsearch_timestamp_check.rb

Line 50 in 00ca651

Time.now.strftime('%Y-%m-%dT%H:%M:%S.%L%z')

AFAIU, every event that is ingested in fluntd has an internal time key, either extracted from the event record (using formatting/parsing) or by using the time when it was ingested, since the event can travel through the pipeline, by the time it reaches this plugin Time.now doesn't represent the actual ingestion time of the event.

Any thoughts? @ecwws @cosmo0920

[cosmo0920]

Because msgpack cannot handle Time object.
In record, users have to insert timestamp as String object or something which contains Time information.
For using elasticsearch, milliseconds time resolution(.%L) is enough.

[dannyk81]

Thanks @cosmo0920, I'm probably missing something, but can't we use this:

Time.at(time).strftime('%Y-%m-%dT%H:%M:%S.%L%z')

Here's the full snippet (lines 48 - 52):

      unless record['fluent_converted_timestamp']
        record['@timestamp'] = record['fluent_added_timestamp'] =
          Time.at(time).strftime('%Y-%m-%dT%H:%M:%S.%L%z')
        $log.debug("Timestamp added: #{record['@timestamp']}")
      end

The filter should have access to tag, time and record ?

[cosmo0920]

The filter should have access to tag, time and record ?

Within #filter, we should access tag, time, and record but we can only modify record.

[dannyk81]

Yes I uderstand, but I'm not proposing to modify the time object, just to derive the values for keys @timestamp & fluent_added_timestamp in the record from time

record['@timestamp'] = record['fluent_added_timestamp'] = Time.at(time).strftime('%Y-%m-%dT%H:%M:%S.%L%z')

This should work isn't it?

[ecwws]

For practical purposes, the difference between event time and the time obtained using Time.now are probably negligible. However, the point you brought up, using time field of the object that was supplied to the plugin does have a certain valid use case: parser plugins that has a setting of time_key will cause the time field to be set using the field from the source data.

Let me test a couple things out, then I'll have it fixed up.

[dannyk81]

Thanks @ecwws 😄

In our deployment, we are using this plugin on the aggregation fluentd nodes, before sending the events to Elasticsearch. We try to keep our fluentd forwarders configuration as basic/simple as possible.

In this kind of setup, since the forwarders have a flush interval of 5 seconds, using Time.now on the aggregation nodes will always introduce a delay in the value of @timestamp compared to the actual ingestion time of the even.

[dannyk81]

@ecwws any luck with this?

[dannyk81]

@ecwws ping 😄

[ecwws]

@dannyk81 sorry, work suddenly got really really busy, I'm going to try to figure this out tonight

[ecwws]

@dannyk81 sorry it took long, 0.2.8 should be using the event time instead of Time.now for added timestamp