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

Add time_series_metric parameter #76766

Merged
merged 17 commits into from
Sep 20, 2021
Merged
Show file tree
Hide file tree
Changes from 6 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
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public class ScriptScoreBenchmark {
private final ScriptModule scriptModule = new ScriptModule(Settings.EMPTY, pluginsService.filterPlugins(ScriptPlugin.class));

private final Map<String, MappedFieldType> fieldTypes = Map.ofEntries(
Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of(), null, false))
Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of(), null, false, null))
);
private final IndexFieldDataCache fieldDataCache = new IndexFieldDataCache.None();
private final CircuitBreakerService breakerService = new NoneCircuitBreakerService();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,27 @@ public TokenCountFieldMapper build(ContentPath contentPath) {

static class TokenCountFieldType extends NumberFieldMapper.NumberFieldType {

TokenCountFieldType(String name, boolean isSearchable, boolean isStored,
boolean hasDocValues, Number nullValue, Map<String, String> meta) {
super(name, NumberFieldMapper.NumberType.INTEGER, isSearchable, isStored, hasDocValues, false, nullValue, meta, null, false);
TokenCountFieldType(
String name,
boolean isSearchable,
boolean isStored,
boolean hasDocValues,
Number nullValue,
Map<String, String> meta
) {
super(
name,
NumberFieldMapper.NumberType.INTEGER,
isSearchable,
isStored,
hasDocValues,
false,
nullValue,
meta,
null,
false,
null
);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,10 @@ public Parameter<T> setValidator(Consumer<T> validator) {
return this;
}

public Consumer<T> getValidator() {
return this.validator;
}

/**
* Configure a custom serializer for this parameter
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.index.fielddata.IndexNumericFieldData.NumericType;
import org.elasticsearch.index.fielddata.plain.SortedNumericIndexFieldData;
import org.elasticsearch.index.mapper.TimeSeriesParams.MetricType;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.script.DoubleFieldScript;
import org.elasticsearch.script.LongFieldScript;
Expand All @@ -57,6 +58,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;

Expand All @@ -83,8 +85,18 @@ public static class Builder extends FieldMapper.Builder {

private final Parameter<Script> script = Parameter.scriptParam(m -> toType(m).builder.script.get());
private final Parameter<String> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);

/**
* Parameter that marks this field as a time series dimension.
*/
private final Parameter<Boolean> dimension;

/**
* Parameter that marks this field as a time series metric defining its time series metric type.
* For the numeric fields gauge and counter metric types are
* supported
*/
private final Parameter<String> metric;

private final Parameter<Map<String, String>> meta = Parameter.metaParam();

Expand Down Expand Up @@ -126,6 +138,25 @@ public Builder(String name, NumberType type, ScriptCompiler compiler, boolean ig
}
});

this.metric = TimeSeriesParams.metricParam(
m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
null,
MetricType.gauge.name(),
MetricType.counter.name()
);
// We are overriding the default validator that checks for acceptable values.
// Therefore, we must call the default validator explicitly from inside our new validator.
// TODO: Make parameters support multiple validators
final Consumer<String> defaultValidator = metric.getValidator();
metric.setValidator(v -> {
// Call the default validator first
defaultValidator.accept(v);

if (v != null && v.isEmpty() == false && hasDocValues.getValue() == false) {
throw new IllegalArgumentException("Field [" + metric.name + "] requires that [" + hasDocValues.name + "] is true");
}
});

this.script.precludesParameters(ignoreMalformed, coerce, nullValue);
addScriptValidation(script, indexed, hasDocValues);
}
Expand All @@ -152,9 +183,26 @@ public Builder dimension(boolean dimension) {
return this;
}

public Builder metric(String metric) {
this.metric.setValue(metric);
return this;
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(indexed, hasDocValues, stored, ignoreMalformed, coerce, nullValue, script, onScriptError, meta, dimension);
return List.of(
indexed,
hasDocValues,
stored,
ignoreMalformed,
coerce,
nullValue,
script,
onScriptError,
meta,
dimension,
metric
);
}

@Override
Expand Down Expand Up @@ -967,26 +1015,29 @@ public static class NumberFieldType extends SimpleMappedFieldType {
private final Number nullValue;
private final FieldValues<Number> scriptValues;
private final boolean isDimension;
private final MetricType metricType;

public NumberFieldType(String name, NumberType type, boolean isSearchable, boolean isStored,
boolean hasDocValues, boolean coerce, Number nullValue, Map<String, String> meta,
FieldValues<Number> script, boolean isDimension) {
FieldValues<Number> script, boolean isDimension, MetricType metricType) {
super(name, isSearchable, isStored, hasDocValues, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta);
this.type = Objects.requireNonNull(type);
this.coerce = coerce;
this.nullValue = nullValue;
this.scriptValues = script;
this.isDimension = isDimension;
this.metricType = metricType;
}

NumberFieldType(String name, Builder builder) {
this(name, builder.type, builder.indexed.getValue(), builder.stored.getValue(), builder.hasDocValues.getValue(),
builder.coerce.getValue().value(), builder.nullValue.getValue(), builder.meta.getValue(),
builder.scriptValues(), builder.dimension.getValue());
builder.scriptValues(), builder.dimension.getValue(),
MetricType.fromString(builder.metric.getValue()));
}

public NumberFieldType(String name, NumberType type) {
this(name, type, true, false, true, true, null, Collections.emptyMap(), null, false);
this(name, type, true, false, true, true, null, Collections.emptyMap(), null, false, null);
}

@Override
Expand Down Expand Up @@ -1086,6 +1137,14 @@ public CollapseType collapseType() {
public boolean isDimension() {
return isDimension;
}

/**
* If field is a time series metric field, returns its metric type
* @return the metric type or null
*/
public MetricType getMetricType() {
return metricType;
}
}

private final Builder builder;
Expand All @@ -1101,6 +1160,7 @@ public boolean isDimension() {
private final boolean ignoreMalformedByDefault;
private final boolean coerceByDefault;
private final boolean dimension;
private final TimeSeriesParams.MetricType metricType;

private NumberFieldMapper(
String simpleName,
Expand All @@ -1120,6 +1180,7 @@ private NumberFieldMapper(
this.coerceByDefault = builder.coerce.getDefaultValue().value();
this.scriptValues = builder.scriptValues();
this.dimension = builder.dimension.getValue();
this.metricType = MetricType.fromString(builder.metric.getValue());
this.builder = builder;
}

Expand Down Expand Up @@ -1208,6 +1269,8 @@ protected void indexScriptValues(SearchLookup searchLookup, LeafReaderContext re
@Override
public FieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName(), type, builder.scriptCompiler, ignoreMalformedByDefault, coerceByDefault)
.dimension(dimension).init(this);
.dimension(dimension)
.metric(metricType != null ? metricType.name() : null)
.init(this);
}
}
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
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.index.mapper;

import java.util.function.Function;

/**
* Utility functions for time series related mapper parameters
*/
public final class TimeSeriesParams {

public static final String TIME_SERIES_METRIC_PARAM = "time_series_metric";

private TimeSeriesParams() {
}

public enum MetricType {
gauge,
counter,
histogram,
summary;

/**
* Convert string to MetricType value returning null for null values
*/
public static MetricType fromString(String name) {
return name != null ? valueOf(name) : null;
}
}

public static FieldMapper.Parameter<String> metricParam(Function<FieldMapper, String> initializer, String... values) {
return FieldMapper.Parameter.restrictedStringParam(TIME_SERIES_METRIC_PARAM, false, initializer, values).acceptsNull();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -311,13 +311,13 @@ private void doTestRequireDocValues(MappedFieldType ft) {
public void testRequireDocValuesOnLongs() {
doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", LONG));
doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", LONG,
true, false, false, false, null, Collections.emptyMap(), null, false));
true, false, false, false, null, Collections.emptyMap(), null, false, null));
}

public void testRequireDocValuesOnDoubles() {
doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", NumberType.DOUBLE));
doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", NumberType.DOUBLE,
true, false, false, false, null, Collections.emptyMap(), null, false));
true, false, false, false, null, Collections.emptyMap(), null, false, null));
}

public void testRequireDocValuesOnBools() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.io.IOException;
import java.util.List;
import java.util.function.Function;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -260,6 +261,59 @@ public void testDimension() throws IOException {
assertThat(e.getCause().getMessage(), containsString("Parameter [dimension] cannot be set"));
}

protected <T> void assertMetricType(String metricType, Function<T, Enum<TimeSeriesParams.MetricType>> checker)
Copy link
Member

Choose a reason for hiding this comment

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

Do you think it'd be worth sticking this in the superclass so we don't have to copy and paste it?

throws IOException {
MapperService mapperService = createMapperService(fieldMapping(b -> {
minimalMapping(b);
b.field("time_series_metric", metricType);
}));

@SuppressWarnings("unchecked") // Syntactic sugar in tests
T fieldType = (T) mapperService.fieldType("field");
assertThat(checker.apply(fieldType).name(), equalTo(metricType));
}

public void testMetricType() throws IOException {
// Test default setting
MapperService mapperService = createMapperService(fieldMapping(b -> minimalMapping(b)));
NumberFieldMapper.NumberFieldType ft = (NumberFieldMapper.NumberFieldType) mapperService.fieldType("field");
assertNull(ft.getMetricType());

assertMetricType("gauge", NumberFieldMapper.NumberFieldType::getMetricType);
assertMetricType("counter", NumberFieldMapper.NumberFieldType::getMetricType);

{
// Test invalid metric type for this field type
Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
minimalMapping(b);
b.field("time_series_metric", "histogram");
})));
assertThat(
e.getCause().getMessage(),
containsString("Unknown value [histogram] for field [time_series_metric] - accepted values are [null, gauge, counter]")
);
}
{
// Test invalid metric type for this field type
Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
minimalMapping(b);
b.field("time_series_metric", "unknown");
})));
assertThat(
e.getCause().getMessage(),
containsString("Unknown value [unknown] for field [time_series_metric] - accepted values are [null, gauge, counter]")
);
}
}

public void testMetricAndDocvalues() {
Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
minimalMapping(b);
b.field("time_series_metric", "counter").field("doc_values", false);
})));
assertThat(e.getCause().getMessage(), containsString("Field [time_series_metric] requires that [doc_values] is true"));
}

@Override
protected final Object generateRandomInputValue(MappedFieldType ft) {
Number n = randomNumber();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public void testLongTermQueryWithDecimalPart() {
}

private static MappedFieldType unsearchable() {
return new NumberFieldType("field", NumberType.LONG, false, false, true, true, null, Collections.emptyMap(), null, false);
return new NumberFieldType("field", NumberType.LONG, false, false, true, true, null, Collections.emptyMap(), null, false, null);
}

public void testTermQuery() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ private ValuesSourceConfig getVSConfig(
null,
Collections.emptyMap(),
null,
false
false,
null
);
return ValuesSourceConfig.resolveFieldOnly(ft, context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,8 @@ public void testDocValuesFieldExistsForNumber() throws IOException {
null,
Map.of(),
null,
false
false,
null
);
docValuesFieldExistsTestCase(new ExistsQueryBuilder("f"), ft, true, i -> {
return numberType.createFields("f", i, true, true, false);
Expand All @@ -1080,7 +1081,8 @@ public void testDocValuesFieldExistsForNumberWithoutData() throws IOException {
null,
Map.of(),
null,
false
false,
null
));
}

Expand Down
Loading