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

Add support for logstash codecs #114

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ToToL
Copy link

@ToToL ToToL commented Jun 16, 2020

Hi !

Add support for executing codec for body of http request.

I don't know why it has not been done before, maybe there is a reason. Let me know if it is.

Regards

@cla-checker-service
Copy link

❌ Author of the following commits did not sign a Contributor Agreement:
078aa89

Please, read and sign the above mentioned agreement if you want to contribute to this project

Copy link
Contributor

@kares kares left a comment

Choose a reason for hiding this comment

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

👍 this is a sold start but in order to ship the feature needs more work:

  • deprecate the format option in favor of the codec setting
  • format => codec looks like a hack - would keep format as is (just mark it obsolete)
  • format default (:default => 'json') should be removed
  • plugin should detect a format configured by user (!= nil) and use it
  • if both codec and format are user configured the plugin should error
    (this is a bit tricky - happy to assist as/if needed when other pieces are done)
  • the new version should be a minor bump

(sorry for the late review)

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.

3 participants