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

Added encoding and from_encoding parameters #17

Merged

Conversation

yoheimuta
Copy link
Contributor

In our production environment, to_json errors have occurred after updating td-agent v2.3.5.

fluentd.warn.internal.message.ip-10-0-1-218
----------------
[15:30:56] Slack Error: /opt/td-agent/embedded/lib/ruby/gems/2.1.0/gems/activesupport-4.2.8/lib/active_support/core_ext/object/json.rb:34:in `encode' / "\xE4" from ASCII-8BIT to UTF-8

This error is when fluent-plugin-slack to_json the output of fluent-plugin-rds-slowlog.
Specifically, in-rds-slowlog outputs ASCII-8BIT encoded character string, but out_slack's to_json expects UTF-8 encoded character string, so it is an error.

The direct cause is that the emitted ASCII-8BIT string is not implicitly converted to UTF-8 by current msgpack-ruby, since msgpack-ruby v0.6.0~ is now internally used by updating td-agent (fluentd)

Since we confirmed that this problem will be solved by passing UTF-8 to encoding parameter, we made PR.

@yoheimuta
Copy link
Contributor Author

continuous-integration/travis-ci/pr

There is no point to think about, is it caused by the correction contents here?

https://travis-ci.org/kenjiskywalker/fluent-plugin-rds-slowlog/jobs/233084029 -- failed
https://travis-ci.org/yoheimuta/fluent-plugin-rds-slowlog/jobs/233084021 -- passed

@cosmo0920
Copy link
Contributor

This PR seems to be useful.
Could you rebase at current master, @yoheimuta ?

@kenjiskywalker How do you think about this PR?

@kenjiskywalker
Copy link
Owner

@cosmo0920
thanks notification and advice 👍

@yoheimuta
sorry for my late reply .
Plz rebase , I'll check it again 🙏

@yoheimuta
Copy link
Contributor Author

Thanks! I'll rebase.

@@ -13,9 +13,34 @@ class Fluent::Plugin::Rds_SlowlogInput < Fluent::Plugin::Input
config_param :password, :string, :default => nil, :secret => true
config_param :interval, :integer, :default => 10
config_param :backup_table, :string, :default => nil
# The encoding after conversion of the input.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is just a my thought. Feel free to ignore it.
We can describe parameters how to work with desc method which can display usage with fluent-plugin-config-format.
So, we might have to add parameter descriptions with desc method.
But this is not issue, just a suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks !
It is desirable to add the desc method, so I added a fdf2ad4.

Copy link
Contributor

Choose a reason for hiding this comment

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

Very Good. Thank you!

@yoheimuta
Copy link
Contributor Author

Thanks! I'll rebase.

@kenjiskywalker @cosmo0920 I rebased, so please confirm.

@kenjiskywalker
Copy link
Owner

@yoheimuta @cosmo0920

Thank you 🙏

@kenjiskywalker kenjiskywalker merged commit 5854bc4 into kenjiskywalker:master Aug 8, 2017
@yoheimuta
Copy link
Contributor Author

Thanks!

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.

3 participants