-
Notifications
You must be signed in to change notification settings - Fork 12
Description
Description
After introducing Bridge API multiple plugin breaks happened and as I do see some extra dependencies were consumed. Or we never PoCed if Bridge wrappers actually work. This issue uses Bridge API as it is and listed all the gaps need to be addressed.
For the visibility, I have created a draft PR which actually build with Bridge API and runs unit/integration test. Unfortunately due to ES main
change the plugin build is broken but I have luckily capture just before this change.
CRITICAL ISSUES
(see the full investigation points below)
For the painless plugin, we had a logic to apply extensions. This was executed through initializing the ScriptService
and as we moved the ScriptService
to the Bridge and simplified (include whitelists, etc..), we no longer need the ScriptService
initialization in the plugin. However, it seems we still need to add the extensions in the bridge
- relative source in the Bridge API
// ScriptServiceBridge.java
public class ScriptServiceBridge extends StableBridgeAPI.Proxy<ScriptService> implements Closeable {
public ScriptServiceBridge wrap(final ScriptService delegate) {
return new ScriptServiceBridge(delegate);
}
public ScriptServiceBridge(final SettingsBridge settingsBridge, final LongSupplier timeProvider) {
super(getScriptService(settingsBridge.unwrap(), timeProvider));
}
public ScriptServiceBridge(ScriptService delegate) {
super(delegate);
}
private static ScriptService getScriptService(final Settings settings, final LongSupplier timeProvider) {
final List<Whitelist> painlessBaseWhitelist = getPainlessBaseWhiteList();
final Map<ScriptContext<?>, List<Whitelist>> scriptContexts = Map.of(
IngestScript.CONTEXT,
painlessBaseWhitelist,
IngestConditionalScript.CONTEXT,
painlessBaseWhitelist
);
final Map<String, ScriptEngine> scriptEngines = Map.of(
PainlessScriptEngine.NAME,
new PainlessScriptEngine(settings, scriptContexts),
MustacheScriptEngine.NAME,
new MustacheScriptEngine(settings)
);
return new ScriptService(settings, scriptEngines, ScriptModule.CORE_CONTEXTS, timeProvider);
}
private static List<Whitelist> getPainlessBaseWhiteList() {
return PainlessPlugin.baseWhiteList();
}
@Override
public void close() throws IOException {
this.delegate.close();
}
- relative source (can be removed) in the plugin
src/main/java/co/elastic/logstash/filters/elasticintegration/EventProcessorBuilder.java
file:
private static ScriptService initScriptService(final Settings settings, final ThreadPool threadPool) throws IOException {
Map<String, ScriptEngine> engines = new HashMap<>();
engines.put(PainlessScriptEngine.NAME, getPainlessScriptEngine(settings));
engines.put(MustacheScriptEngine.NAME, new MustacheScriptEngine(settings));
return new ScriptService(settings, engines, ScriptModule.CORE_CONTEXTS, threadPool::absoluteTimeInMillis);
}
/**
* @param settings the Elasticsearch settings object
* @return a {@link ScriptEngine} for painless scripts for use in {@link IngestScript} and
* {@link IngestConditionalScript} contexts, including all available {@link PainlessExtension}s.
* @throws IOException when the underlying script engine cannot be created
*/
private static ScriptEngine getPainlessScriptEngine(final Settings settings) throws IOException {
try (final PainlessPlugin painlessPlugin = new PainlessPlugin()) {
painlessPlugin.loadExtensions(new ExtensiblePlugin.ExtensionLoader() {
@Override
@SuppressWarnings("unchecked")
public <T> List<T> loadExtensions(Class<T> extensionPointType) {
if (extensionPointType.isAssignableFrom(PainlessExtension.class)) {
final List<PainlessExtension> extensions = new ArrayList<>();
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
return (List<T>) extensions;
} else {
return List.of();
}
}
});
return painlessPlugin.getScriptEngine(settings, Set.of(IngestScript.CONTEXT, IngestConditionalScript.CONTEXT));
}
}
Investigation
- see the CRITICAL ISSUE above
- introduce module descriptors for the plugins which makes them available to the logstash-bridge
ProjectId
: it recently came into play (with Apply upstream ES GeoIP factory interface changes. #273)- it seems we can safely remove this dependency as
null
was applied for it: https://github.com/elastic/elasticsearch/blob/main/libs/logstash-bridge/src/main/java/org/elasticsearch/logstashbridge/ingest/PipelineBridge.java#L37 (the PR) - however, for the long term apply
ProjectId.DEFAULT
insteadnull
- it seems we can safely remove this dependency as
org.elasticsearch.core.Releasable
(used byIntegrationBatch.java
) dependency: it seems it was missedorg.elasticsearch.action.support.RefCountingRunnable
(used byEventProcessor.java
) dependency: it seems it was missedorg.elasticsearch.ingest.common.FailProcessorException
dependency: it seems it was missed- removed
org.elasticsearch.core.Strings.format
dependency and the same logic behindorg.elasticsearch.core.Strings.format
can be simply applied withString.format(Locale.ROOT, args)
IngestDocument.Metadata.VERSION
was referenced in the plugin but we do not have a wrapper in the bridge for this enum- Processors:
- all processors are directly consumed in the plugins, we need to move them to the ES or do they stay? - My concern is plugin behaviour (particular processor support or not) should be controlled by the plugin not by ES imho.
RedactProcessor
with its license (XPackLicenseState
) dependant is not coveredSetSecurityUserProcessor
can be removed, the PR - Make to behave all unsupported processors in the same way. #269