painless: register ProcessorsWhitelistExtension to painless engine#162
painless: register ProcessorsWhitelistExtension to painless engine#162yaauie merged 4 commits intoelastic:mainfrom
Conversation
| } | ||
|
|
||
| private static ScriptService initScriptService(final Settings settings, final ThreadPool threadPool) { | ||
| final List<Whitelist> painlessBaseWhitelist = getPainlessBaseWhiteList(); |
There was a problem hiding this comment.
review note: by using PainlessPlugin#getScriptEngine(Settings, Set<ScriptContext>) instead of `new PainlessScriptEngine(Settings, Map<ScriptContext<?>, List>), we no longer need to manage the whole whitelists ourselves.
| painlessPlugin.loadExtensions(new ExtensiblePlugin.ExtensionLoader() { | ||
| @Override | ||
| @SuppressWarnings("unchecked") | ||
| public <T> List<T> loadExtensions(Class<T> extensionPointType) { | ||
| if (extensionPointType == PainlessExtension.class) { | ||
| return List.of((T) new ProcessorsWhitelistExtension()); | ||
| } else { | ||
| return List.of(); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
review note: a single-use ExtensiblePlugin.ExtensionLoader is common-place in Elasticsearch core and looks very similar to this.
While we know that PainlessPlugin#loadExtensions(ExtensiblePlugin.ExtensionLoader) will only ever send ExtensiblePlugin.ExtensionLoader#loadExtensions(PainlessExtension.class), we need to accept any class in order to satisfy the interface.
Enables the use of the `org.elasticsearch.ingest.common.Processors` in ingest pipelines, as is found in integrations like `logs-aws.cloudtrail`. Fixes: elastic#161
40841c6 to
b288d6a
Compare
| extensions.add(new ConstantKeywordPainlessExtension()); // module: constant-keyword | ||
| extensions.add(new ProcessorsWhitelistExtension()); // module: ingest-common | ||
| extensions.add(new SpatialPainlessExtension()); // module: spatial | ||
| extensions.add(new WildcardPainlessExtension()); // module: wildcard |
There was a problem hiding this comment.
@mashhurs you brought up on a separate channel that we should audit to see what other painless extensions need to be registered, so I went through all of the implementations of PainlessExtension in ES source to find those that worked with either the IngestScript.CONTEXT or IngestConditionalScript.CONTEXT (and would thus be useful to our ScriptEngine instance that only supports those two contexts):
PainlessExtension |
Script Context | Source |
|---|---|---|
AggregationsPainlessExtension |
❌context-miss | -- |
AnalysisPainlessExtension |
❌context-miss | -- |
ConstantKeywordPainlessExtension |
✅context-match | ✅constant-keyword |
DocValuesWhitelistExtension |
❌context-miss | -- |
EqlPainlessExtension |
❌context-miss | -- |
MachineLearningPainlessExtension |
❌context-miss | -- |
Murmur3PainlessExtension |
✅context-match | ❌not included |
ProcessorsWhitelistExtension |
✅context-match | ✅ingest-common |
RuntimeFieldsPainlessExtension |
❌context-miss | -- |
SpatialPainlessExtension |
✅context-match | ✅spatial |
SqlPainlessExtension |
❌context-miss | -- |
VersionFieldDocValuesExtension |
❌context-miss | -- |
WatcherPainlessExtension |
❌context-miss | -- |
WildcardPainlessExtension |
✅context-match | ✅wildcard |
The murmur3 painless plugin would work, but from what I can tell it isn't included in the ES default distribution.
There was a problem hiding this comment.
Thank you @yaauie for the details, makes sense.
…telist-proccessors Add an integration test for the painless script which points to Ingest Processors.
mashhurs
left a comment
There was a problem hiding this comment.
LGTM!
Side notes:
- Validated with yaauie#1 and local test.
- 🔴 CIs are not due to this change (build script is resolving 8.16 branch, which doesn't exist, will fix). It is 🟢 with latest release and anyway have issue with
mainbranch (see: https://buildkite.com/elastic/logstash-filter-elastic-integration-build/builds/334)
| extensions.add(new ConstantKeywordPainlessExtension()); // module: constant-keyword | ||
| extensions.add(new ProcessorsWhitelistExtension()); // module: ingest-common | ||
| extensions.add(new SpatialPainlessExtension()); // module: spatial | ||
| extensions.add(new WildcardPainlessExtension()); // module: wildcard |
There was a problem hiding this comment.
Thank you @yaauie for the details, makes sense.
💔 Build Failed
Failed CI Steps
History
|
Enables the use of the
org.elasticsearch.ingest.common.Processorsin ingest pipelines, as is found in integrations likelogs-aws.cloudtrail.Fixes: #161
Thanks for contributing to Logstash! If you haven't already signed our CLA, here's a handy link: https://www.elastic.co/contributor-agreement/