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

Fix TestDriver to let plugin instance always to have its class name #1069

Merged
merged 3 commits into from Aug 15, 2016

Conversation

sonots
Copy link
Member

@sonots sonots commented Jun 30, 2016

Noticed via fluent/fluent-plugin-s3#154

v0.14 TestLogger currently outputs as:

2011-01-03 13:07:36 +0000 [warn]: [] out_s3: delayed events were put

when a TestDriver is created with a block.

v0.14 TestLogger tries to add optional_header like [#{self.class.name}]

From: fluent/fluentd/lib/fluent/log.rb @ line 426 Fluent::PluginLoggerMixin#configure:

    422: def configure(conf)
    423:   super

    428:   if level = conf['@log_level']
    429:     unless @log.is_a?(PluginLogger)
    430:       @log = PluginLogger.new($log.dup)
    431:     end
    432:     @log.level = level
    433:     @log.optional_header = "[#{self.class.name}#{plugin_id_configured? ? "(" + @id + ")" : ""}] "
    434:     @log.optional_attrs = {}
    435:   end
    436: end

but, TestDriver creates plugin instance with Class.new if block is given, therefore, it does not have class name. In such case we get the extra [].

With this fix, we will get [Fluent::S3Output] as we expect.

@tagomoris
Copy link
Member

tagomoris commented Jul 4, 2016

LGTM, but it's missed to fix for v0.14 test drivers.

@tagomoris
Copy link
Member

@sonots ping?

@sonots
Copy link
Member Author

sonots commented Jul 11, 2016

Modified similarly ParserTestDriver and FormatterTestDriver

@tagomoris
Copy link
Member

I meant that the same fix is needed for v0.14 plugin test drivers under fluent/test/driver.

@tagomoris tagomoris added the incomplete Missing version, reproducible steps, etc label Jul 25, 2016
@sonots
Copy link
Member Author

sonots commented Jul 25, 2016

Sorry to be late. Fixed for test drivers under fluent/test/driver, too.

@tagomoris tagomoris removed the incomplete Missing version, reproducible steps, etc label Aug 15, 2016
@tagomoris
Copy link
Member

LGTM.

@tagomoris tagomoris merged commit bd4b3cf into master Aug 15, 2016
@repeatedly repeatedly deleted the fix_test_driver branch March 9, 2018 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants