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

Backport PR #13244 to 8.1: i18n: alias logstash.runner.configuration.* under logstash.agent.configuration #13842

Merged
merged 1 commit into from
Mar 18, 2022

Conversation

yaauie
Copy link
Member

@yaauie yaauie commented Mar 4, 2022

Backport PR #13244 to 8.1 branch. Original message:

A number of plugins reach into Logstash's i18n translations to "helpfully" communicate certain configuration errors, but rely on translations that were moved in #3872 from logstash.agent to logstash.runner. Since then, it is possible to hit obtuse error messages about failing to load a translation instead of the intended helpful message:

translation missing: en.logstash.agent.configuration.invalid_plugin_register

By moving the logstash.agent definition to after the logstash.runner definition, we can use YAML tooling to name the logstash.runner.configuration node and then merge its contents into logstash.agent.configuration. This effectively allows us to keep a single definition of those translations while making them available at both addresses.

Release notes

Fixed an issue where some improperly-configured plugins would attempt to provide helpful guidance but fail to load the correct help-text.

What does this PR do?

Aliases each of the translation keys in the en.logstash.runner.configuration into en.logstash.agent.configuration so that plugins that still rely on the old location of those help-texts will be able to find them.

Why is it important/What is the impact to the user?

Turns an obtuse error like:

(ConfigurationError) translation missing: en.logstash.agent.configuration.invalid_plugin_register

Int a more helpful one like:

 (ConfigurationError) Cannot register filter FILTER_NAME plugin. The error reported is:\n  ERROR_DESCRIPTION"

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files (and/or docker env variables)
  • [ ] I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

The Date Filter is one of many plugins that uses Logstash's i18n and attempts to lookup the logstash.agent.configuration. invalid_plugin_register when provided incorrectly-shaped input. Looking into Logstash's vendor directory will yield many more. We can intentionally hit one of these like:

bin/logstash -e 'filter { date { match => "this_is_wrong" } }'

Without this patch, our error is an obtuse one about failing to look up a translation, and with this patch the error is marginally more helpful.

…iguration (elastic#13244)

A number of plugins reach into Logstash's i18n translations to "helpfully"
communicate certain configuration errors, but rely on translations that were
moved in 00a99c1 from logstash.agent to
logstash.runner. Since then, it is possible to hit obtuse error messages about
failing to load a translation instead of the intended helpful message:

~~~
translation missing: en.logstash.agent.configuration.invalid_plugin_register
~~~

By moving the `logstash.agent` definition to _after_ the `logstash.runner`
definition, we can use YAML tooling to name the `logstash.runner.configuration`
node and then merge its contents into `logstash.agent.configuration`. This
effectively allows us to keep a single definition of those translations while
making them available at both addresses.

Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com>
(cherry picked from commit 8bec0e6)
Copy link
Contributor

@kaisecheng kaisecheng left a comment

Choose a reason for hiding this comment

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

LGTM

@jsvd jsvd merged commit bebd064 into elastic:8.1 Mar 18, 2022
@jsvd jsvd added v8.1.2 and removed v8.1.1 labels Mar 28, 2022
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.

4 participants