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

Symmetric time parse and format #1207

Merged
merged 16 commits into from
Sep 6, 2016
Merged

Conversation

tagomoris
Copy link
Member

This patch is to provide consistent API about time and tag, handling these on records.

Provided things:

  • symmetric class structure: Fluent::TimeFormatter and Fluent::TimeParser with #call method (and #parse/#format)
  • consistent API to parse / format time with configurable format and timezone
  • common configuration parameters for parser/formatter plugins and inject/extract plugin helpers

With this patch, formatter plugins can get TimeFormatter instances anytime via time_formatter_create method with configured format/timezone by users in <format> section.
Parser plugins also does it via time_parser_create.
Input/Output plugins can use inject/extract plugin helpers with just same configuration parameter names. It provides consistent how-to-use for users.

@tagomoris
Copy link
Member Author

@repeatedly could you review this change?

config_param :time_key, :string, default: nil
config_param :time_type, :enum, list: [:float, :unixtime, :string], default: :float

Fluent::TimeMixin::TIME_PARAMETERS.each do |name, type, opts|
Copy link
Member

Choose a reason for hiding this comment

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

Why don't you use include Fluent::TimeMixin::TimeParameters?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because that mixin adds parameters on the top namespace, not in <extract> sections.

Copy link
Member

Choose a reason for hiding this comment

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

I see. implementation limitation...

@repeatedly
Copy link
Member

Off topic. Cache machanizm of parser / formatter becomes overhead for msec / nsec times.
We should improve this point, e,g separate TimeParser into TimeParser and CachedTimeParser, in the future.

@tagomoris tagomoris added v0.14 enhancement Feature request or improve operations labels Sep 6, 2016
Fluent::Test.setup
end

def test_call_with_parse
Copy link
Member

Choose a reason for hiding this comment

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

What does 'call' mean?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no idea: it's just moved from test/plugin/test_parser_time.rb to there.

@repeatedly
Copy link
Member

Other LGTM.

@tagomoris tagomoris merged commit d8a40cd into master Sep 6, 2016
@tagomoris tagomoris deleted the symmetric-time-parse-and-format branch September 6, 2016 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request or improve operations v0.14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants