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

EQL: Add integration tests harness to test EQL feature parity with original implementation #52248

Merged
merged 12 commits into from
Feb 22, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
30 changes: 17 additions & 13 deletions x-pack/plugin/eql/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,23 @@ ext {

archivesBaseName = 'x-pack-eql'

test {
testLogging.showStandardStreams = true
}

// All integration tests live in qa modules
integTest.enabled = false

// Instead we create a separate task to run the tests based on ESIntegTestCase
task internalClusterTest(type: Test) {
testLogging.showStandardStreams = true
mustRunAfter test
include '**/*IT.class'
systemProperty 'es.set.netty.runtime.available.processors', 'false'
}

check.dependsOn internalClusterTest

dependencies {
compileOnly project(path: xpackModule('core'), configuration: 'default')
compileOnly(project(':modules:lang-painless')) {
Expand All @@ -33,19 +50,6 @@ dependencies {
testCompile project(path: ':modules:analysis-common', configuration: 'runtime')
}

integTest.enabled = false
testingConventions.enabled = false

// Instead we create a separate task to run the tests based on ESIntegTestCase
task internalClusterTest(type: Test) {
description = '🌈🌈🌈🦄 Welcome to fantasy integration tests land! 🦄🌈🌈🌈'
mustRunAfter test

include '**/*IT.class'
systemProperty 'es.set.netty.runtime.available.processors', 'false'
}

check.dependsOn internalClusterTest

/****************************************************************
* Enable QA/rest integration tests for snapshot builds only *
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ setup:

- match: {timed_out: false}
- match: {took: 0}
- match: {hits.total.value: 0}
- match: {hits.total.value: 1}

Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

package org.elasticsearch.xpack.eql.action;

import org.elasticsearch.action.ActionRequestBuilder;
import org.elasticsearch.client.ElasticsearchClient;
import org.elasticsearch.index.query.QueryBuilder;

public class EqlSearchRequestBuilder extends ActionRequestBuilder<EqlSearchRequest, EqlSearchResponse> {
public EqlSearchRequestBuilder(ElasticsearchClient client, EqlSearchAction action) {
super(client, action, new EqlSearchRequest());
}

public EqlSearchRequestBuilder indices(String... indices) {
request.indices(indices);
return this;
}

public EqlSearchRequestBuilder query(QueryBuilder query) {
request.query(query);
return this;
}

public EqlSearchRequestBuilder timestampField(String timestampField) {
request.timestampField(timestampField);
return this;
}

public EqlSearchRequestBuilder eventTypeField(String eventTypeField) {
request.eventTypeField(eventTypeField);
return this;
}

public EqlSearchRequestBuilder implicitJoinKeyField(String implicitJoinKeyField) {
request.implicitJoinKeyField(implicitJoinKeyField);
return this;
}

public EqlSearchRequestBuilder fetchSize(int size) {
request.fetchSize(size);
return this;
}

public EqlSearchRequestBuilder searchAfter(Object[] values) {
request.searchAfter(values);
return this;
}

public EqlSearchRequestBuilder rule(String rule) {
request.rule(rule);
return this;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -483,5 +483,21 @@ public boolean equals(Object o) {
public int hashCode() {
return Objects.hash(events, sequences, counts, totalHits);
}

public List<SearchHit> events() {
return this.events;
}

public List<Sequence> sequences() {
return this.sequences;
}

public List<Count> counts() {
return this.counts;
}

public TotalHits totalHits() {
return this.totalHits;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.plugins.ActionPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestHandler;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.watcher.ResourceWatcherService;
import org.elasticsearch.xpack.core.XPackPlugin;
import org.elasticsearch.xpack.core.action.XPackInfoFeatureAction;
import org.elasticsearch.xpack.core.action.XPackUsageFeatureAction;
import org.elasticsearch.xpack.eql.EqlInfoTransportAction;
Expand All @@ -39,12 +41,13 @@

import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.function.Supplier;

public class EqlPlugin extends Plugin implements ActionPlugin {

private final boolean enabled;

private static final boolean EQL_FEATURE_FLAG_REGISTERED;

static {
Expand All @@ -69,6 +72,10 @@ public class EqlPlugin extends Plugin implements ActionPlugin {
Setting.Property.NodeScope
);

public EqlPlugin(final Settings settings) {
this.enabled = EQL_ENABLED_SETTING.get(settings);
}

@Override
public Collection<Object> createComponents(Client client, ClusterService clusterService, ThreadPool threadPool,
ResourceWatcherService resourceWatcherService, ScriptService scriptService, NamedXContentRegistry xContentRegistry,
Expand All @@ -83,17 +90,6 @@ private Collection<Object> createComponents(Client client, String clusterName, N
return Arrays.asList(planExecutor);
}


@Override
public List<ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
return Arrays.asList(
new ActionHandler<>(EqlSearchAction.INSTANCE, TransportEqlSearchAction.class),
new ActionHandler<>(EqlStatsAction.INSTANCE, TransportEqlStatsAction.class),
new ActionHandler<>(XPackUsageFeatureAction.EQL, EqlUsageTransportAction.class),
new ActionHandler<>(XPackInfoFeatureAction.EQL, EqlInfoTransportAction.class)
);
}

/**
* The settings defined by EQL plugin.
*
Expand All @@ -103,9 +99,21 @@ private Collection<Object> createComponents(Client client, String clusterName, N
public List<Setting<?>> getSettings() {
Copy link
Member

Choose a reason for hiding this comment

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

Either we use enabled or this method. On one hand enabled checks the property presence statically but this method tries to do it dynamic when a class is instantiated.
There's there are multiple pieces of isSnapshot in this class that need simplification.

Copy link
Member Author

Choose a reason for hiding this comment

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

tried to follow the autoscaling plugin pattern, that I was told should be an example of handling the feature flag.

if (isSnapshot() || EQL_FEATURE_FLAG_REGISTERED) {
return List.of(EQL_ENABLED_SETTING);
} else {
return List.of();
}
return List.of();
}

@Override
public List<ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused about why the method was moved...

Copy link
Member Author

Choose a reason for hiding this comment

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

consistent with other plugins. can undo

if (enabled) {
return List.of(
new ActionHandler<>(EqlSearchAction.INSTANCE, TransportEqlSearchAction.class),
new ActionHandler<>(EqlStatsAction.INSTANCE, TransportEqlStatsAction.class),
new ActionHandler<>(XPackUsageFeatureAction.EQL, EqlUsageTransportAction.class),
new ActionHandler<>(XPackInfoFeatureAction.EQL, EqlInfoTransportAction.class)
);
}
return List.of();
}

boolean isSnapshot() {
Expand All @@ -126,9 +134,12 @@ public List<RestHandler> getRestHandlers(Settings settings,
IndexNameExpressionResolver indexNameExpressionResolver,
Supplier<DiscoveryNodes> nodesInCluster) {

if (isEnabled(settings) == false) {
return Collections.emptyList();
if (enabled) {
return List.of(new RestEqlSearchAction(), new RestEqlStatsAction());
}
return Arrays.asList(new RestEqlSearchAction(), new RestEqlStatsAction());
return List.of();
}

// overridable by tests
protected XPackLicenseState getLicenseState() { return XPackPlugin.getSharedLicenseState(); }
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@

import java.time.ZoneId;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

public class TransportEqlSearchAction extends HandledTransportAction<EqlSearchRequest, EqlSearchResponse> {
Expand Down Expand Up @@ -63,12 +62,12 @@ public static void operation(PlanExecutor planExecutor, EqlSearchRequest request
TimeValue timeout = TimeValue.timeValueSeconds(30);
boolean includeFrozen = request.indicesOptions().ignoreThrottled() == false;
String clientId = null;

ParserParams params = new ParserParams()
.fieldEventType(request.eventTypeField())
.fieldTimestamp(request.timestampField())
.implicitJoinKey(request.implicitJoinKeyField());

Configuration cfg = new Configuration(request.indices(), zoneId, username, clusterName, filter, timeout, includeFrozen, clientId);
//planExecutor.eql(cfg, request.rule(), params, wrap(r -> listener.onResponse(createResponse(r)), listener::onFailure));
listener.onResponse(createResponse(null));
Expand All @@ -77,14 +76,14 @@ public static void operation(PlanExecutor planExecutor, EqlSearchRequest request
static EqlSearchResponse createResponse(Results results) {
// Stubbed search response
// TODO: implement actual search response processing once the parser/executor is in place
// Updated for stubbed response to: process where serial_event_id = 1
// to validate the sample test until the engine is wired in.
List<SearchHit> events = Arrays.asList(
new SearchHit(1, "111", null),
new SearchHit(2, "222", null)
new SearchHit(1, "111", null)
);
EqlSearchResponse.Hits hits = new EqlSearchResponse.Hits(null, Arrays.asList(
new EqlSearchResponse.Sequence(Collections.singletonList("4021"), events),
new EqlSearchResponse.Sequence(Collections.singletonList("2343"), events)
), null, new TotalHits(0, TotalHits.Relation.EQUAL_TO));
EqlSearchResponse.Hits hits = new EqlSearchResponse.Hits(events, null,
null, new TotalHits(1, TotalHits.Relation.EQUAL_TO));

return new EqlSearchResponse(hits, 0, false);
}

Expand All @@ -95,4 +94,4 @@ static String username(SecurityContext securityContext) {
static String clusterName(ClusterService clusterService) {
return clusterService.getClusterName().value();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@

package org.elasticsearch.xpack.eql;

import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.xpack.eql.session.Configuration;
import org.elasticsearch.xpack.ql.util.StringUtils;

import static org.elasticsearch.test.ESTestCase.randomAlphaOfLength;
import static org.elasticsearch.test.ESTestCase.randomBoolean;
Expand All @@ -31,4 +33,17 @@ public static Configuration randomConfiguration() {
randomBoolean(),
randomAlphaOfLength(16));
}

public static Tuple<String, String> pathAndName(String string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this method used anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

No longer used indeed, will remove

String folder = StringUtils.EMPTY;
String file = string;
int lastIndexOf = string.lastIndexOf("/");
if (lastIndexOf > 0) {
folder = string.substring(0, lastIndexOf - 1);
if (lastIndexOf + 1 < string.length()) {
file = string.substring(lastIndexOf + 1);
}
}
return new Tuple<>(folder, file);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.eql.action;

import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.LicenseService;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.eql.plugin.EqlPlugin;

import java.util.Collection;
import java.util.Collections;

import static org.elasticsearch.test.ESIntegTestCase.Scope.SUITE;

@ESIntegTestCase.ClusterScope(scope = SUITE, numDataNodes = 0, numClientNodes = 0, maxNumDataNodes = 0)
public abstract class AbstractEqlIntegTestCase extends ESIntegTestCase {

@Override
protected Settings nodeSettings(int nodeOrdinal) {
Settings.Builder settings = Settings.builder().put(super.nodeSettings(nodeOrdinal));
settings.put(XPackSettings.SECURITY_ENABLED.getKey(), false);
Copy link
Contributor

Choose a reason for hiding this comment

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

At one point we'll have to test EQL in a security enabled context as well, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably. Was following AbstractSqlIntegTestCase as example in this case.

settings.put(XPackSettings.MONITORING_ENABLED.getKey(), false);
settings.put(XPackSettings.WATCHER_ENABLED.getKey(), false);
settings.put(XPackSettings.GRAPH_ENABLED.getKey(), false);
settings.put(XPackSettings.MACHINE_LEARNING_ENABLED.getKey(), false);
settings.put(EqlPlugin.EQL_ENABLED_SETTING.getKey(), true);
settings.put(LicenseService.SELF_GENERATED_LICENSE_TYPE.getKey(), "trial");
return settings.build();
}

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Collections.singletonList(LocalStateEqlXPackPlugin.class);
}
}

Loading