-
Notifications
You must be signed in to change notification settings - Fork 12
Make to behave all unsupported processors in the same way. #269
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
base: main
Are you sure you want to change the base?
Conversation
@@ -33,7 +32,6 @@ | |||
import org.elasticsearch.painless.PainlessPlugin; | |||
import org.elasticsearch.painless.PainlessScriptEngine; | |||
import org.elasticsearch.painless.spi.PainlessExtension; | |||
import org.elasticsearch.painless.spi.Whitelist; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not used anywhere.
This pull request does not have a backport label. Could you fix it @mashhurs? 🙏
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need a rebase, but I approve it in principle. Please tag me so I can rubber-stamp it after the rebase has been done and the build is green.
@@ -128,7 +126,6 @@ public EventProcessorBuilder() { | |||
org.elasticsearch.ingest.common.UriPartsProcessor.TYPE)); | |||
this.addProcessorsFromPlugin(IngestUserAgentPlugin::new); | |||
this.addProcessorsFromPlugin(RedactPlugin::new); | |||
this.addProcessor(SetSecurityUserProcessor.TYPE, SetSecurityUserProcessor.Factory::new); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any code references to set_security_user
in elastic/integrations, and it has never been in our published list of supported processors.
I am +1 to deleting it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, we have unsupported processors scan automation and so far not appeared.
…ocessors same behavior. # Conflicts: # src/main/java/co/elastic/logstash/filters/elasticintegration/ingest/SetSecurityUserProcessor.java
d02c7ff
to
5b0b57e
Compare
💚 Build Succeeded
History
|
Description
Removes
set_security_user
processor which silently succeeds without telling its unsupported processor.The plugin behavior for unsupported processors will be same with this change.
Adds integration tests to make sure all unsupported processors behave in the same way.
Logs
before:
inference
orenrich
processor will be tagged with_ingest_pipeline_failure
and error in the LS logsset_security_user
will be processed without issue but no data will be set to the event - this is we don't want.with this change: all events will be tagged with
_ingest_pipeline_failure
if unsupported processors in the ingest pipeline, errors can be seen in the LS logs: