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

Register Fluent::Plugin::RegexpParser as parser plugin #1094

Merged
merged 10 commits into from
Jul 19, 2016

Conversation

okkez
Copy link
Contributor

@okkez okkez commented Jul 11, 2016

No description provided.

@okkez
Copy link
Contributor Author

okkez commented Jul 11, 2016

I want to use Fluent::Plugin::RegexpParser as a parser plugin.
I'm trying to migrate in_tail to v0.14 API.

We can write configuration as following, after merged this PR.

<source>
  @type tail
  <parse>
    @type regexp
    expression /pattern/
  </parse>
</source>

We can use parser_create helper method as following.

@parser = parser_create(type: "regexp", conf: {"expression" => "/pattern/"})

@tagomoris
Copy link
Member

Good fix. But don't use Compat modules in v0.14 lib/fluent/plugin directory.
How about to create new regexp-based plugins like this?

module Fluent::Plugin
  class NginxParser < RegexpParser
    Fluent::Plugin.register_parser('nginx', self)
    config_set_default :expression, "....."
  end
end

@okkez
Copy link
Contributor Author

okkez commented Jul 12, 2016

I've pushed changes using Fluent::Plugin::RegexpParser.
But still remains Compat modules in parser_multiline.rb

@tagomoris
Copy link
Member

Are there any specific problems for parser_multiline.rb to be solved?

@okkez
Copy link
Contributor Author

okkez commented Jul 12, 2016

For now, Fluent::Plugin::RegexpParser cannot accept Regexp::MULTILINE option or Regexp object.

@okkez
Copy link
Contributor Author

okkez commented Jul 13, 2016

For now, Fluent::Plugin::RegexpParser cannot accept Regexp::MULTILINE option or Regexp object.

I've added new config_param to Fluent::Plugin::RegexpParser and replace Compat module in parser_multiline.rb.
Could you revriew again? @tagomoris


class ApacheErrorParserTest < ::Test::Unit::TestCase
def setup
Fluent::Test.setup
@parser = Fluent::Test::Driver::Parser.new(Fluent::Plugin.new_parser('apache_error'))
@parser = Fluent::Test::Driver::Parser.new(Fluent::Plugin::ApacheErrorParser.new)
Copy link
Member

Choose a reason for hiding this comment

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

This instance is not parser, but test driver instance. I know it's too late to notify about it, but I want not to leave it as-is because it's misleading for following developers.
@okkez Could you fix it?

@tagomoris
Copy link
Member

I added comments only for test code (and it originally came from other changes).
Others looks good to me.

@okkez
Copy link
Contributor Author

okkez commented Jul 19, 2016

I've fixed and pushed.

@tagomoris
Copy link
Member

Thank you! I'll merge this later.

@tagomoris tagomoris merged commit cd65e42 into fluent:master Jul 19, 2016
@okkez okkez deleted the extract-regexp-parser branch August 18, 2016 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants