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

ingest: date_index_name processor template resolution #31841

Merged
merged 3 commits into from
Jul 11, 2018
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import org.elasticsearch.ingest.ConfigurationUtils;
import org.elasticsearch.ingest.IngestDocument;
import org.elasticsearch.ingest.Processor;
import org.elasticsearch.ingest.ValueSource;
import org.elasticsearch.script.ScriptService;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.format.DateTimeFormat;
Expand All @@ -47,16 +49,18 @@ public final class DateIndexNameProcessor extends AbstractProcessor {
private final String indexNameFormat;
private final DateTimeZone timezone;
private final List<Function<String, DateTime>> dateFormats;
private final ScriptService scriptService;

DateIndexNameProcessor(String tag, String field, List<Function<String, DateTime>> dateFormats, DateTimeZone timezone,
String indexNamePrefix, String dateRounding, String indexNameFormat) {
String indexNamePrefix, String dateRounding, String indexNameFormat, ScriptService scriptService) {
super(tag);
this.field = field;
this.timezone = timezone;
this.dateFormats = dateFormats;
this.indexNamePrefix = indexNamePrefix;
this.dateRounding = dateRounding;
this.indexNameFormat = indexNameFormat;
this.scriptService = scriptService;
}

@Override
Expand Down Expand Up @@ -94,7 +98,7 @@ public void execute(IngestDocument ingestDocument) throws Exception {
.append('}')
.append('>');
String dynamicIndexName = builder.toString();
ingestDocument.setFieldValue(IngestDocument.MetaData.INDEX.getFieldName(), dynamicIndexName);
ingestDocument.setFieldValue(IngestDocument.MetaData.INDEX.getFieldName(), ValueSource.wrap(dynamicIndexName, scriptService));
Copy link
Member

Choose a reason for hiding this comment

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

I think it is better that the indexNamePrefix, dateRounding and indexNameFormat become TemplateScript.Factory typed fields, like in other processor implementations. ValueSource is more meant to template generic values and in this case we know the templated values are always strings. Also then it is not needed to pass down ScriptService into the processor implementation and then the change in IngestDocument is not needed too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@martijnvg - thanks for the review ! the changes requested are at f9a6ec1. This PR should be ready for re-review.

}

@Override
Expand Down Expand Up @@ -128,6 +132,12 @@ List<Function<String, DateTime>> getDateFormats() {

public static final class Factory implements Processor.Factory {

private final ScriptService scriptService;

public Factory(ScriptService scriptService) {
this.scriptService = scriptService;
}

@Override
public DateIndexNameProcessor create(Map<String, Processor.Factory> registry, String tag,
Map<String, Object> config) throws Exception {
Expand Down Expand Up @@ -156,7 +166,8 @@ public DateIndexNameProcessor create(Map<String, Processor.Factory> registry, St
String indexNamePrefix = ConfigurationUtils.readStringProperty(TYPE, tag, config, "index_name_prefix", "");
String dateRounding = ConfigurationUtils.readStringProperty(TYPE, tag, config, "date_rounding");
String indexNameFormat = ConfigurationUtils.readStringProperty(TYPE, tag, config, "index_name_format", "yyyy-MM-dd");
return new DateIndexNameProcessor(tag, field, dateFormats, timezone, indexNamePrefix, dateRounding, indexNameFormat);
return new DateIndexNameProcessor(tag, field, dateFormats, timezone, indexNamePrefix,
dateRounding, indexNameFormat, scriptService);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public Map<String, Processor.Factory> getProcessors(Processor.Parameters paramet
processors.put(GsubProcessor.TYPE, new GsubProcessor.Factory());
processors.put(FailProcessor.TYPE, new FailProcessor.Factory(parameters.scriptService));
processors.put(ForEachProcessor.TYPE, new ForEachProcessor.Factory());
processors.put(DateIndexNameProcessor.TYPE, new DateIndexNameProcessor.Factory());
processors.put(DateIndexNameProcessor.TYPE, new DateIndexNameProcessor.Factory(parameters.scriptService));
processors.put(SortProcessor.TYPE, new SortProcessor.Factory());
processors.put(GrokProcessor.TYPE, new GrokProcessor.Factory(GROK_PATTERNS, createGrokThreadWatchdog(parameters)));
processors.put(ScriptProcessor.TYPE, new ScriptProcessor.Factory(parameters.scriptService));
Expand All @@ -97,12 +97,12 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
Supplier<DiscoveryNodes> nodesInCluster) {
return Arrays.asList(new GrokProcessorGetAction.RestAction(settings, restController));
}

@Override
public List<Setting<?>> getSettings() {
return Arrays.asList(WATCHDOG_INTERVAL, WATCHDOG_MAX_EXECUTION_TIME);
}

private static ThreadWatchdog createGrokThreadWatchdog(Processor.Parameters parameters) {
long intervalMillis = WATCHDOG_INTERVAL.get(parameters.env.settings()).getMillis();
long maxExecutionTimeMillis = WATCHDOG_MAX_EXECUTION_TIME.get(parameters.env.settings()).getMillis();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.ingest.common;

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.ingest.TestTemplateService;
import org.elasticsearch.test.ESTestCase;
import org.hamcrest.Matchers;
import org.joda.time.DateTimeZone;
Expand All @@ -31,7 +32,7 @@
public class DateIndexNameFactoryTests extends ESTestCase {

public void testDefaults() throws Exception {
DateIndexNameProcessor.Factory factory = new DateIndexNameProcessor.Factory();
DateIndexNameProcessor.Factory factory = new DateIndexNameProcessor.Factory(TestTemplateService.instance());
Map<String, Object> config = new HashMap<>();
config.put("field", "_field");
config.put("date_rounding", "y");
Expand All @@ -46,7 +47,7 @@ public void testDefaults() throws Exception {
}

public void testSpecifyOptionalSettings() throws Exception {
DateIndexNameProcessor.Factory factory = new DateIndexNameProcessor.Factory();
DateIndexNameProcessor.Factory factory = new DateIndexNameProcessor.Factory(TestTemplateService.instance());
Map<String, Object> config = new HashMap<>();
config.put("field", "_field");
config.put("index_name_prefix", "_prefix");
Expand Down Expand Up @@ -84,7 +85,7 @@ public void testSpecifyOptionalSettings() throws Exception {
}

public void testRequiredFields() throws Exception {
DateIndexNameProcessor.Factory factory = new DateIndexNameProcessor.Factory();
DateIndexNameProcessor.Factory factory = new DateIndexNameProcessor.Factory(TestTemplateService.instance());
Map<String, Object> config = new HashMap<>();
config.put("date_rounding", "y");
ElasticsearchParseException e = expectThrows(ElasticsearchParseException.class, () -> factory.create(null, null, config));
Expand All @@ -95,5 +96,4 @@ public void testRequiredFields() throws Exception {
e = expectThrows(ElasticsearchParseException.class, () -> factory.create(null, null, config));
assertThat(e.getMessage(), Matchers.equalTo("[date_rounding] required property is missing"));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,34 @@
package org.elasticsearch.ingest.common;

import org.elasticsearch.ingest.IngestDocument;
import org.elasticsearch.ingest.TestTemplateService;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.TemplateScript;
import org.elasticsearch.test.ESTestCase;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.mockito.Matchers;
import org.mockito.Mockito;

import java.util.Collections;
import java.util.Locale;
import java.util.function.Function;

import static org.elasticsearch.script.Script.DEFAULT_TEMPLATE_LANG;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class DateIndexNameProcessorTests extends ESTestCase {

public void testJodaPattern() throws Exception {
Function<String, DateTime> function = DateFormat.Joda.getFunction("yyyy-MM-dd'T'HH:mm:ss.SSSZ", DateTimeZone.UTC, Locale.ROOT);
DateIndexNameProcessor processor = new DateIndexNameProcessor(
"_tag", "_field", Collections.singletonList(function), DateTimeZone.UTC,
"events-", "y", "yyyyMMdd"
"events-", "y", "yyyyMMdd", TestTemplateService.instance()
);

IngestDocument document = new IngestDocument("_index", "_type", "_id", null, null, null,
Expand All @@ -47,7 +58,7 @@ public void testJodaPattern() throws Exception {
public void testTAI64N()throws Exception {
Function<String, DateTime> function = DateFormat.Tai64n.getFunction(null, DateTimeZone.UTC, null);
DateIndexNameProcessor dateProcessor = new DateIndexNameProcessor("_tag", "_field", Collections.singletonList(function),
DateTimeZone.UTC, "events-", "m", "yyyyMMdd");
DateTimeZone.UTC, "events-", "m", "yyyyMMdd", TestTemplateService.instance());
IngestDocument document = new IngestDocument("_index", "_type", "_id", null, null, null,
Collections.singletonMap("_field", (randomBoolean() ? "@" : "") + "4000000050d506482dbdf024"));
dateProcessor.execute(document);
Expand All @@ -57,7 +68,7 @@ public void testTAI64N()throws Exception {
public void testUnixMs()throws Exception {
Function<String, DateTime> function = DateFormat.UnixMs.getFunction(null, DateTimeZone.UTC, null);
DateIndexNameProcessor dateProcessor = new DateIndexNameProcessor("_tag", "_field", Collections.singletonList(function),
DateTimeZone.UTC, "events-", "m", "yyyyMMdd");
DateTimeZone.UTC, "events-", "m", "yyyyMMdd", TestTemplateService.instance());
IngestDocument document = new IngestDocument("_index", "_type", "_id", null, null, null,
Collections.singletonMap("_field", "1000500"));
dateProcessor.execute(document);
Expand All @@ -72,11 +83,28 @@ public void testUnixMs()throws Exception {
public void testUnix()throws Exception {
Function<String, DateTime> function = DateFormat.Unix.getFunction(null, DateTimeZone.UTC, null);
DateIndexNameProcessor dateProcessor = new DateIndexNameProcessor("_tag", "_field", Collections.singletonList(function),
DateTimeZone.UTC, "events-", "m", "yyyyMMdd");
DateTimeZone.UTC, "events-", "m", "yyyyMMdd", TestTemplateService.instance());
IngestDocument document = new IngestDocument("_index", "_type", "_id", null, null, null,
Collections.singletonMap("_field", "1000.5"));
dateProcessor.execute(document);
assertThat(document.getSourceAndMetadata().get("_index"), equalTo("<events-{19700101||/m{yyyyMMdd|UTC}}>"));
}

public void testTemplateSupported() throws Exception {
ScriptService scriptService = mock(ScriptService.class);
TestTemplateService.MockTemplateScript.Factory factory = new TestTemplateService.MockTemplateScript.Factory("script_result");
when(scriptService.compile(any(Script.class), Matchers.<ScriptContext<TemplateScript.Factory>>any())).thenReturn(factory);
when(scriptService.isLangSupported(DEFAULT_TEMPLATE_LANG)).thenReturn(true);

DateIndexNameProcessor dateProcessor = new DateIndexNameProcessor("_tag", "_field",
Collections.singletonList(DateFormat.Unix.getFunction(null, DateTimeZone.UTC, null)),
DateTimeZone.UTC, "events-", "m", "yyyyMMdd", scriptService);
IngestDocument document = new IngestDocument("_index", "_type", "_id", null, null, null,
Collections.singletonMap("_field", "1000.5"));
dateProcessor.execute(document);

// here we only care that the script was compiled and that it returned what we expect.
Mockito.verify(scriptService).compile(any(Script.class), Matchers.<ScriptContext<TemplateScript.Factory>>any());
assertThat(document.getSourceAndMetadata().get("_index"), equalTo("script_result"));
}
}
15 changes: 15 additions & 0 deletions server/src/main/java/org/elasticsearch/ingest/IngestDocument.java
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,21 @@ public void setFieldValue(String path, Object value) {
setFieldValue(path, value, false);
}

/**
* Sets the provided value to the provided path in the document.
* Any non existing path element will be created.
* If the last item in the path is a list, the value will replace the existing list as a whole.
* Use {@link #appendFieldValue(String, Object)} to append values to lists instead.
* @param path The path within the document in dot-notation
* @param valueSource The value source that will produce the value or values to append to the existing ones
* @throws IllegalArgumentException if the path is null, empty, invalid or if the value cannot be set to the
* item identified by the provided path.
*/
public void setFieldValue(String path, ValueSource valueSource) {
Map<String, Object> model = valueSource == null ? null : createTemplateModel();
setFieldValue(path, valueSource == null ? null : valueSource.copyAndResolve(model), false);
}

/**
* Sets the provided value to the provided path in the document.
* Any non existing path element will be created. If the last element is a list,
Expand Down