Skip to content

Allow ESQL extra verifiers to access settings #129954

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

Merged
merged 11 commits into from
Jun 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 0 additions & 29 deletions x-pack/plugin/esql/qa/server/extra-checkers/build.gradle

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
}

public static class EsqlTestPluginWithMockBlockFactory extends EsqlPlugin {
public EsqlTestPluginWithMockBlockFactory(Settings settings) {
super(settings);
}

@Override
protected BlockFactoryProvider blockFactoryProvider(
CircuitBreaker breaker,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

package org.elasticsearch.xpack.esql.action;

import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.License;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.license.internal.XPackLicenseStatus;
Expand All @@ -19,6 +20,10 @@
* that require an Enteprise (or Trial) license.
*/
public class EsqlPluginWithEnterpriseOrTrialLicense extends EsqlPlugin {
public EsqlPluginWithEnterpriseOrTrialLicense(Settings settings) {
super(settings);
}

protected XPackLicenseState getLicenseState() {
License.OperationMode operationMode = randomFrom(License.OperationMode.ENTERPRISE, License.OperationMode.TRIAL);
return new XPackLicenseState(() -> System.currentTimeMillis(), new XPackLicenseStatus(operationMode, true, "Test license expired"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

package org.elasticsearch.xpack.esql.action;

import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.License;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.license.internal.XPackLicenseStatus;
Expand All @@ -22,6 +23,10 @@
* - an expired enterprise or trial license
*/
public class EsqlPluginWithNonEnterpriseOrExpiredLicense extends EsqlPlugin {
public EsqlPluginWithNonEnterpriseOrExpiredLicense(Settings settings) {
super(settings);
}

protected XPackLicenseState getLicenseState() {
License.OperationMode operationMode;
boolean active;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

package org.elasticsearch.xpack.esql.spatial;

import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.License;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.license.internal.XPackLicenseStatus;
Expand Down Expand Up @@ -49,6 +50,10 @@ private static XPackLicenseState getLicenseState() {
* This is used to test the behavior of spatial functions when no valid license is present.
*/
public static class TestEsqlPlugin extends EsqlPlugin {
public TestEsqlPlugin(Settings settings) {
super(settings);
}

protected XPackLicenseState getLicenseState() {
return SpatialNoLicenseTestCase.getLicenseState();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

package org.elasticsearch.xpack.esql.analysis;

import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.xpack.esql.LicenseAware;
import org.elasticsearch.xpack.esql.capabilities.PostAnalysisPlanVerificationAware;
Expand Down Expand Up @@ -60,7 +61,7 @@ public interface ExtraCheckers {
* Build a list of checks to perform on the plan. Each one is called once per
* {@link LogicalPlan} node in the plan.
*/
List<BiConsumer<LogicalPlan, Failures>> extra();
List<BiConsumer<LogicalPlan, Failures>> extra(Settings settings);
}

/**
Expand All @@ -69,15 +70,17 @@ public interface ExtraCheckers {
private final List<ExtraCheckers> extraCheckers;
private final Metrics metrics;
private final XPackLicenseState licenseState;
private final Settings settings;

public Verifier(Metrics metrics, XPackLicenseState licenseState) {
this(metrics, licenseState, Collections.emptyList());
this(metrics, licenseState, Collections.emptyList(), Settings.EMPTY);
}

public Verifier(Metrics metrics, XPackLicenseState licenseState, List<ExtraCheckers> extraCheckers) {
public Verifier(Metrics metrics, XPackLicenseState licenseState, List<ExtraCheckers> extraCheckers, Settings settings) {
this.metrics = metrics;
this.licenseState = licenseState;
this.extraCheckers = extraCheckers;
this.settings = settings;
}

/**
Expand All @@ -102,7 +105,7 @@ Collection<Failure> verify(LogicalPlan plan, BitSet partialMetrics) {
// collect plan checkers
var planCheckers = planCheckers(plan);
for (ExtraCheckers e : extraCheckers) {
planCheckers.addAll(e.extra());
planCheckers.addAll(e.extra(settings));
}

// Concrete verifications
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package org.elasticsearch.xpack.esql.execution;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.indices.IndicesExpressionGrouper;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.telemetry.metric.MeterRegistry;
Expand Down Expand Up @@ -52,14 +53,15 @@ public PlanExecutor(
MeterRegistry meterRegistry,
XPackLicenseState licenseState,
EsqlQueryLog queryLog,
List<Verifier.ExtraCheckers> extraCheckers
List<Verifier.ExtraCheckers> extraCheckers,
Settings settings
) {
this.indexResolver = indexResolver;
this.preAnalyzer = new PreAnalyzer();
this.functionRegistry = new EsqlFunctionRegistry();
this.mapper = new Mapper();
this.metrics = new Metrics(functionRegistry);
this.verifier = new Verifier(metrics, licenseState, extraCheckers);
this.verifier = new Verifier(metrics, licenseState, extraCheckers, settings);
this.planTelemetryManager = new PlanTelemetryManager(meterRegistry);
this.queryLog = queryLog;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ public class EsqlPlugin extends Plugin implements ActionPlugin, ExtensiblePlugin
);

private final List<Verifier.ExtraCheckers> extraCheckers = new ArrayList<>();
private final Settings settings;

public EsqlPlugin(Settings settings) {
this.settings = settings;
}

@Override
public Collection<?> createComponents(PluginServices services) {
Expand All @@ -209,7 +214,8 @@ public Collection<?> createComponents(PluginServices services) {
services.telemetryProvider().getMeterRegistry(),
getLicenseState(),
new EsqlQueryLog(services.clusterService().getClusterSettings(), services.slowLogFieldProvider()),
extraCheckers
extraCheckers,
settings
),
new ExchangeService(
services.clusterService().getSettings(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package org.elasticsearch.xpack.esql.action;

import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.compute.operator.Operator;
import org.elasticsearch.compute.operator.topn.TopNOperatorStatus;
import org.elasticsearch.test.ESTestCase;
Expand All @@ -18,7 +19,7 @@
public class NamedWriteablesTests extends ESTestCase {

public void testTopNStatus() throws Exception {
try (EsqlPlugin plugin = new EsqlPlugin()) {
try (EsqlPlugin plugin = new EsqlPlugin(Settings.EMPTY)) {
NamedWriteableRegistry registry = new NamedWriteableRegistry(plugin.getNamedWriteables());
TopNOperatorStatus origin = new TopNOperatorStatus(
randomNonNegativeInt(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ protected Writeable.Reader<ClusterComputeRequest> instanceReader() {
protected NamedWriteableRegistry getNamedWriteableRegistry() {
List<NamedWriteableRegistry.Entry> writeables = new ArrayList<>();
writeables.addAll(new SearchModule(Settings.EMPTY, List.of()).getNamedWriteables());
writeables.addAll(new EsqlPlugin().getNamedWriteables());
writeables.addAll(new EsqlPlugin(Settings.EMPTY).getNamedWriteables());
return new NamedWriteableRegistry(writeables);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ protected Writeable.Reader<DataNodeRequest> instanceReader() {
protected NamedWriteableRegistry getNamedWriteableRegistry() {
List<NamedWriteableRegistry.Entry> writeables = new ArrayList<>();
writeables.addAll(new SearchModule(Settings.EMPTY, List.of()).getNamedWriteables());
writeables.addAll(new EsqlPlugin().getNamedWriteables());
writeables.addAll(new EsqlPlugin(Settings.EMPTY).getNamedWriteables());
return new NamedWriteableRegistry(writeables);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,14 @@ public void testFailedMetric() {
return null;
}).when(esqlClient).execute(eq(EsqlResolveFieldsAction.TYPE), any(), any());

var planExecutor = new PlanExecutor(indexResolver, MeterRegistry.NOOP, new XPackLicenseState(() -> 0L), mockQueryLog(), List.of());
var planExecutor = new PlanExecutor(
indexResolver,
MeterRegistry.NOOP,
new XPackLicenseState(() -> 0L),
mockQueryLog(),
List.of(),
Settings.EMPTY
);
var enrichResolver = mockEnrichResolver();

var request = new EsqlQueryRequest();
Expand Down