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

Preserve grok pattern ordering and add sort option #61671

Merged
merged 5 commits into from
Sep 8, 2020
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
3 changes: 2 additions & 1 deletion libs/grok/src/main/java/org/elasticsearch/grok/Grok.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
Expand Down Expand Up @@ -288,7 +289,7 @@ private static Map<String, String> loadBuiltinPatterns() throws IOException {
"java", "junos", "linux-syslog", "maven", "mcollective-patterns", "mongodb", "nagios",
"postgresql", "rails", "redis", "ruby", "squid"
};
Map<String, String> builtinPatterns = new HashMap<>();
Map<String, String> builtinPatterns = new LinkedHashMap<>();
for (String pattern : PATTERN_NAMES) {
try(InputStream is = Grok.class.getResourceAsStream("/patterns/" + pattern)) {
loadPatterns(builtinPatterns, is);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.elasticsearch.ingest.common;

import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
Expand All @@ -40,6 +41,7 @@
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;

import static org.elasticsearch.ingest.common.IngestCommonPlugin.GROK_PATTERNS;
import static org.elasticsearch.rest.RestRequest.Method.GET;
Expand All @@ -55,16 +57,33 @@ private GrokProcessorGetAction() {

public static class Request extends ActionRequest {

public Request() {}
private final boolean sorted;

public Request(boolean sorted) {
this.sorted = sorted;
}

Request(StreamInput in) throws IOException {
super(in);
this.sorted = in.getVersion().onOrAfter(Version.V_8_0_0) ? in.readBoolean() : false;
}

@Override
public ActionRequestValidationException validate() {
return null;
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
out.writeBoolean(sorted);
}
}

public boolean sorted() {
return sorted;
}
}

public static class Response extends ActionResponse implements ToXContentObject {
Expand Down Expand Up @@ -100,15 +119,25 @@ public void writeTo(StreamOutput out) throws IOException {

public static class TransportAction extends HandledTransportAction<Request, Response> {

private final Map<String, String> grokPatterns;
private final Map<String, String> sortedGrokPatterns;

@Inject
public TransportAction(TransportService transportService, ActionFilters actionFilters) {
this(transportService, actionFilters, GROK_PATTERNS);
}

// visible for testing
TransportAction(TransportService transportService, ActionFilters actionFilters, Map<String, String> grokPatterns) {
super(NAME, transportService, actionFilters, Request::new);
this.grokPatterns = grokPatterns;
this.sortedGrokPatterns = new TreeMap<>(this.grokPatterns);
}

@Override
protected void doExecute(Task task, Request request, ActionListener<Response> listener) {
try {
listener.onResponse(new Response(GROK_PATTERNS));
listener.onResponse(new Response(request.sorted() ? sortedGrokPatterns : grokPatterns));
} catch (Exception e) {
listener.onFailure(e);
}
Expand All @@ -129,7 +158,9 @@ public String getName() {

@Override
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) {
return channel -> client.executeLocally(INSTANCE, new Request(), new RestToXContentListener<>(channel));
boolean sorted = request.paramAsBoolean("s", false);
Request grokPatternsRequest = new Request(sorted);
return channel -> client.executeLocally(INSTANCE, grokPatternsRequest, new RestToXContentListener<>(channel));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package org.elasticsearch.ingest.common;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.io.stream.StreamInput;
Expand All @@ -27,18 +29,25 @@
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.transport.TransportService;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;

import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.sameInstance;
import static org.hamcrest.core.IsNull.notNullValue;
import static org.hamcrest.core.IsNull.nullValue;
import static org.mockito.Mockito.mock;

public class GrokProcessorGetActionTests extends ESTestCase {
private static final Map<String, String> TEST_PATTERNS = Collections.singletonMap("PATTERN", "foo");
private static final Map<String, String> TEST_PATTERNS = Map.of("PATTERN2", "foo2", "PATTERN1", "foo1");

public void testRequest() throws Exception {
GrokProcessorGetAction.Request request = new GrokProcessorGetAction.Request();
GrokProcessorGetAction.Request request = new GrokProcessorGetAction.Request(false);
BytesStreamOutput out = new BytesStreamOutput();
request.writeTo(out);
StreamInput streamInput = out.bytes().streamInput();
Expand All @@ -56,15 +65,53 @@ public void testResponseSerialization() throws Exception {
assertThat(response.getGrokPatterns(), equalTo(otherResponse.getGrokPatterns()));
}

public void testResponseSorting() {
List<String> sortedKeys = new ArrayList<>(TEST_PATTERNS.keySet());
Collections.sort(sortedKeys);
GrokProcessorGetAction.TransportAction transportAction =
new GrokProcessorGetAction.TransportAction(mock(TransportService.class), mock(ActionFilters.class), TEST_PATTERNS);
GrokProcessorGetAction.Response[] receivedResponse = new GrokProcessorGetAction.Response[1];
transportAction.doExecute(null, new GrokProcessorGetAction.Request(true), new ActionListener<>() {
@Override
public void onResponse(GrokProcessorGetAction.Response response) {
receivedResponse[0] = response;
}

@Override
public void onFailure(Exception e) {
fail();
}
});
assertThat(receivedResponse[0], notNullValue());
assertThat(receivedResponse[0].getGrokPatterns().keySet().toArray(), equalTo(sortedKeys.toArray()));

GrokProcessorGetAction.Response firstResponse = receivedResponse[0];
transportAction.doExecute(null, new GrokProcessorGetAction.Request(true), new ActionListener<>() {
@Override
public void onResponse(GrokProcessorGetAction.Response response) {
receivedResponse[0] = response;
}

@Override
public void onFailure(Exception e) {
fail();
}
});
assertThat(receivedResponse[0], notNullValue());
assertThat(receivedResponse[0], not(sameInstance(firstResponse)));
assertThat(receivedResponse[0].getGrokPatterns(), sameInstance(firstResponse.getGrokPatterns()));
}

@SuppressWarnings("unchecked")
public void testResponseToXContent() throws Exception {
GrokProcessorGetAction.Response response = new GrokProcessorGetAction.Response(TEST_PATTERNS);
try (XContentBuilder builder = JsonXContent.contentBuilder()) {
response.toXContent(builder, ToXContent.EMPTY_PARAMS);
Map<String, Object> converted = XContentHelper.convertToMap(BytesReference.bytes(builder), false, builder.contentType()).v2();
Map<String, String> patterns = (Map<String, String>) converted.get("patterns");
assertThat(patterns.size(), equalTo(1));
assertThat(patterns.get("PATTERN"), equalTo("foo"));
assertThat(patterns.size(), equalTo(2));
assertThat(patterns.get("PATTERN1"), equalTo("foo1"));
assertThat(patterns.get("PATTERN2"), equalTo("foo2"));
}
}
}