Skip to content

Commit f1cb017

Browse files
JonasKunzJonasKunz
authored
Refactor: Extract exponential histogram parser into analytics plugin (elastic#136668)
1 parent 6f41cb9 commit f1cb017

File tree

12 files changed

+671
-377
lines changed

12 files changed

+671
-377
lines changed

‎benchmarks/src/main/java/org/elasticsearch/benchmark/exponentialhistogram/ExponentialHistogramMergeBench.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
import org.elasticsearch.exponentialhistogram.ExponentialHistogramCircuitBreaker;
1717
import org.elasticsearch.exponentialhistogram.ExponentialHistogramGenerator;
1818
import org.elasticsearch.exponentialhistogram.ExponentialHistogramMerger;
19+
import org.elasticsearch.xpack.analytics.mapper.IndexWithCount;
1920
import org.elasticsearch.xpack.exponentialhistogram.CompressedExponentialHistogram;
20-
import org.elasticsearch.xpack.exponentialhistogram.IndexWithCount;
2121
import org.openjdk.jmh.annotations.Benchmark;
2222
import org.openjdk.jmh.annotations.BenchmarkMode;
2323
import org.openjdk.jmh.annotations.Fork;

‎x-pack/plugin/analytics/build.gradle‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@ dependencies {
2424
clusterPlugins project(':x-pack:plugin:analytics')
2525

2626
api 'org.apache.commons:commons-math3:3.6.1'
27+
implementation(project(":libs:exponential-histogram"))
2728
compileOnly project(path: xpackModule('core'))
2829
compileOnly project(":server")
2930
testImplementation project(path: ':modules:aggregations')
3031
testImplementation(testArtifact(project(xpackModule('core'))))
32+
testImplementation(testArtifact(project(":libs:exponential-histogram")))
3133
}
3234

3335
if (buildParams.snapshotBuild == false) {

‎x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/ExponentialHistogramParser.java‎

Lines changed: 369 additions & 0 deletions
Large diffs are not rendered by default.

x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/IndexWithCount.java renamed to x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/IndexWithCount.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* 2.0.
66
*/
77

8-
package org.elasticsearch.xpack.exponentialhistogram;
8+
package org.elasticsearch.xpack.analytics.mapper;
99

1010
import org.elasticsearch.exponentialhistogram.CopyableBucketIterator;
1111
import org.elasticsearch.exponentialhistogram.ExponentialHistogram;
@@ -21,7 +21,7 @@
2121
*/
2222
public record IndexWithCount(long index, long count) {
2323

24-
static ExponentialHistogram.Buckets asBuckets(int scale, List<IndexWithCount> bucketIndices) {
24+
public static ExponentialHistogram.Buckets asBuckets(int scale, List<IndexWithCount> bucketIndices) {
2525
return new ExponentialHistogram.Buckets() {
2626
@Override
2727
public CopyableBucketIterator iterator() {

‎x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/ExponentialHistogramParserTests.java‎

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.analytics.mapper;
9+
10+
import org.elasticsearch.common.ParsingException;
11+
import org.elasticsearch.common.Strings;
12+
import org.elasticsearch.exponentialhistogram.ExponentialHistogram;
13+
import org.elasticsearch.exponentialhistogram.ExponentialHistogramTestUtils;
14+
import org.elasticsearch.exponentialhistogram.ExponentialHistogramXContent;
15+
import org.elasticsearch.index.mapper.DocumentParsingException;
16+
import org.elasticsearch.test.ESTestCase;
17+
import org.elasticsearch.xcontent.XContentBuilder;
18+
import org.elasticsearch.xcontent.XContentFactory;
19+
import org.elasticsearch.xcontent.XContentParser;
20+
import org.elasticsearch.xcontent.XContentParserConfiguration;
21+
import org.elasticsearch.xcontent.XContentType;
22+
23+
import java.io.IOException;
24+
import java.util.List;
25+
26+
import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MAX_INDEX;
27+
import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MAX_SCALE;
28+
import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MIN_INDEX;
29+
import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MIN_SCALE;
30+
import static org.hamcrest.Matchers.containsString;
31+
import static org.hamcrest.Matchers.equalTo;
32+
33+
public class ExponentialHistogramParserTests extends ESTestCase {
34+
35+
private static final String TEST_FIELD_NAME = "test_field";
36+
37+
public void testParseRandomHistogram() throws IOException {
38+
ExponentialHistogram histogram = ExponentialHistogramTestUtils.randomHistogram();
39+
40+
ExponentialHistogramParser.ParsedExponentialHistogram parsed;
41+
try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
42+
ExponentialHistogramXContent.serialize(builder, histogram);
43+
String json = Strings.toString(builder);
44+
parsed = doParse(json);
45+
}
46+
47+
List<IndexWithCount> expectedPositiveBuckets = IndexWithCount.fromIterator(histogram.positiveBuckets().iterator());
48+
List<IndexWithCount> expectedNegativeBuckets = IndexWithCount.fromIterator(histogram.negativeBuckets().iterator());
49+
50+
assertThat(parsed.scale(), equalTo(histogram.scale()));
51+
52+
assertThat(parsed.zeroThreshold(), equalTo(histogram.zeroBucket().zeroThreshold()));
53+
assertThat(parsed.zeroCount(), equalTo(histogram.zeroBucket().count()));
54+
55+
assertThat(parsed.positiveBuckets(), equalTo(expectedPositiveBuckets));
56+
assertThat(parsed.negativeBuckets(), equalTo(expectedNegativeBuckets));
57+
58+
assertThat(parsed.sum(), equalTo(histogram.sum()));
59+
assertThat(parsed.min(), equalTo(Double.isNaN(histogram.min()) ? null : histogram.min()));
60+
assertThat(parsed.max(), equalTo(Double.isNaN(histogram.max()) ? null : histogram.max()));
61+
}
62+
63+
public void testParseScaleMissing() {
64+
String json = "{}";
65+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
66+
assertThat(ex.getMessage(), containsString("expected field called [scale]"));
67+
}
68+
69+
public void testParseScaleNotNumber() {
70+
String json = "{\"scale\":\"foo\"}";
71+
ParsingException ex = expectThrows(ParsingException.class, () -> doParse(json));
72+
assertThat(ex.getMessage(), containsString("Failed to parse object: expecting token of type [VALUE_NUMBER]"));
73+
}
74+
75+
public void testParseScaleTooLow() {
76+
String json = "{\"scale\":" + (MIN_SCALE - 1) + "}";
77+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
78+
assertThat(
79+
ex.getMessage(),
80+
containsString("scale field must be in range [" + MIN_SCALE + ", " + MAX_SCALE + "] but got " + (MIN_SCALE - 1))
81+
);
82+
}
83+
84+
public void testParseScaleTooHigh() {
85+
String json = "{\"scale\":" + (MAX_SCALE + 1) + "}";
86+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
87+
assertThat(
88+
ex.getMessage(),
89+
containsString("scale field must be in range [" + MIN_SCALE + ", " + MAX_SCALE + "] but got " + (MAX_SCALE + 1))
90+
);
91+
}
92+
93+
public void testParseZeroNotObject() {
94+
String json = "{\"scale\":0,\"zero\":\"not_an_object\"}";
95+
ParsingException ex = expectThrows(ParsingException.class, () -> doParse(json));
96+
assertThat(ex.getMessage(), containsString("Failed to parse object: expecting token of type [START_OBJECT]"));
97+
}
98+
99+
public void testParseZeroThresholdNotNumber() {
100+
String json = "{\"scale\":0,\"zero\":{\"threshold\":\"not_a_number\"}}";
101+
ParsingException ex = expectThrows(ParsingException.class, () -> doParse(json));
102+
assertThat(ex.getMessage(), containsString("Failed to parse object: expecting token of type [VALUE_NUMBER]"));
103+
}
104+
105+
public void testParseZeroThresholdNegative() {
106+
String json = "{\"scale\":0,\"zero\":{\"threshold\":-1.0}}";
107+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
108+
assertThat(ex.getMessage(), containsString("zero.threshold field must be a non-negative, finite number but got -1.0"));
109+
}
110+
111+
public void testParseZeroCountNotNumber() {
112+
String json = "{\"scale\":0,\"zero\":{\"count\":\"not_a_number\"}}";
113+
ParsingException ex = expectThrows(ParsingException.class, () -> doParse(json));
114+
assertThat(ex.getMessage(), containsString("Failed to parse object: expecting token of type [VALUE_NUMBER]"));
115+
}
116+
117+
public void testParseZeroCountNegative() {
118+
String json = "{\"scale\":0,\"zero\":{\"count\":-1}}";
119+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
120+
assertThat(ex.getMessage(), containsString("zero.count field must be a non-negative number but got -1"));
121+
}
122+
123+
public void testParseZeroUnknownField() {
124+
String json = "{\"scale\":0,\"zero\":{\"unknown_field\":123}}";
125+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
126+
assertThat(ex.getMessage(), containsString("with unknown parameter for zero sub-object [unknown_field]"));
127+
}
128+
129+
public void testParsePositiveNotObject() {
130+
String json = "{\"scale\":0,\"positive\":\"not_an_object\"}";
131+
ParsingException ex = expectThrows(ParsingException.class, () -> doParse(json));
132+
assertThat(ex.getMessage(), containsString("Failed to parse object: expecting token of type [START_OBJECT]"));
133+
}
134+
135+
public void testParseNegativeNotObject() {
136+
String json = "{\"scale\":0,\"negative\":\"not_an_object\"}";
137+
ParsingException ex = expectThrows(ParsingException.class, () -> doParse(json));
138+
assertThat(ex.getMessage(), containsString("Failed to parse object: expecting token of type [START_OBJECT]"));
139+
}
140+
141+
public void testParseIndicesNotArray() {
142+
String json = "{\"scale\":0,\"positive\":{\"indices\":\"not_an_array\"}}";
143+
ParsingException ex = expectThrows(ParsingException.class, () -> doParse(json));
144+
assertThat(ex.getMessage(), containsString("Failed to parse object: expecting token of type [START_ARRAY]"));
145+
}
146+
147+
public void testParseCountsNotArray() {
148+
String json = "{\"scale\":0,\"positive\":{\"counts\":\"not_an_array\"}}";
149+
ParsingException ex = expectThrows(ParsingException.class, () -> doParse(json));
150+
assertThat(ex.getMessage(), containsString("Failed to parse object: expecting token of type [START_ARRAY]"));
151+
}
152+
153+
public void testParseIndicesElementNotNumber() {
154+
String json = "{\"scale\":0,\"positive\":{\"indices\":[\"not_a_number\"],\"counts\":[1]}}";
155+
ParsingException ex = expectThrows(ParsingException.class, () -> doParse(json));
156+
assertThat(ex.getMessage(), containsString("Failed to parse object: expecting token of type [VALUE_NUMBER]"));
157+
}
158+
159+
public void testParseCountsElementNotNumber() {
160+
String json = "{\"scale\":0,\"positive\":{\"indices\":[1],\"counts\":[\"not_a_number\"]}}";
161+
ParsingException ex = expectThrows(ParsingException.class, () -> doParse(json));
162+
assertThat(ex.getMessage(), containsString("Failed to parse object: expecting token of type [VALUE_NUMBER]"));
163+
}
164+
165+
public void testParseIndicesBelowMin() {
166+
String json = "{\"scale\":0,\"positive\":{\"indices\":[" + (MIN_INDEX - 1) + "],\"counts\":[1]}}";
167+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
168+
assertThat(
169+
ex.getMessage(),
170+
containsString("positive.indices values must all be in range [" + MIN_INDEX + ", " + MAX_INDEX + "] but got " + (MIN_INDEX - 1))
171+
);
172+
}
173+
174+
public void testParseIndicesAboveMax() {
175+
String json = "{\"scale\":0,\"positive\":{\"indices\":[" + (MAX_INDEX + 1) + "],\"counts\":[1]}}";
176+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
177+
assertThat(
178+
ex.getMessage(),
179+
containsString("positive.indices values must all be in range [" + MIN_INDEX + ", " + MAX_INDEX + "] but got " + (MAX_INDEX + 1))
180+
);
181+
}
182+
183+
public void testParseDuplicateIndices() {
184+
String json = "{\"scale\":0,\"positive\":{\"indices\":[1,1],\"counts\":[1,2]}}";
185+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
186+
assertThat(ex.getMessage(), containsString("expected entries of [positive.indices] to be unique, but got 1 multiple times"));
187+
}
188+
189+
public void testParseCountsZero() {
190+
String json = "{\"scale\":0,\"positive\":{\"indices\":[1],\"counts\":[0]}}";
191+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
192+
assertThat(ex.getMessage(), containsString("positive.counts values must all be greater than zero but got 0"));
193+
}
194+
195+
public void testParseCountsNegative() {
196+
String json = "{\"scale\":0,\"positive\":{\"indices\":[1],\"counts\":[-1]}}";
197+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
198+
assertThat(ex.getMessage(), containsString("positive.counts values must all be greater than zero but got -1"));
199+
}
200+
201+
public void testParseCountsIndicesLengthsDiffer() {
202+
String json = "{\"scale\":0,\"positive\":{\"indices\":[1,2],\"counts\":[1]}}";
203+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
204+
assertThat(ex.getMessage(), containsString("expected same length from [positive.indices] and [positive.counts] but got [2 != 1]"));
205+
}
206+
207+
public void testParseUnknownFieldInPositiveSubObject() {
208+
String json = "{\"scale\":0,\"positive\":{\"unknown_field\":123}}";
209+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
210+
assertThat(ex.getMessage(), containsString("with unknown parameter for positive sub-object [unknown_field]"));
211+
}
212+
213+
public void testParseUnknownFieldInNegativeSubObject() {
214+
String json = "{\"scale\":0,\"negative\":{\"unknown_field\":123}}";
215+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
216+
assertThat(ex.getMessage(), containsString("with unknown parameter for negative sub-object [unknown_field]"));
217+
}
218+
219+
public void testParseUnknownTopLevelField() {
220+
String json = "{\"scale\":0,\"unknown_field\":123}";
221+
DocumentParsingException ex = expectThrows(DocumentParsingException.class, () -> doParse(json));
222+
assertThat(ex.getMessage(), containsString("with unknown parameter [unknown_field]"));
223+
}
224+
225+
private static ExponentialHistogramParser.ParsedExponentialHistogram doParse(String json) throws IOException {
226+
try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, json)) {
227+
parser.nextToken();
228+
parser.nextToken(); // skip START_OBJECT token
229+
return ExponentialHistogramParser.parse(TEST_FIELD_NAME, parser);
230+
}
231+
}
232+
233+
}

‎x-pack/plugin/mapper-exponential-histogram/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ restResources {
2525
}
2626
dependencies {
2727
api project(":libs:exponential-histogram")
28+
api project(xpackModule('analytics'))
2829
compileOnly project(path: xpackModule('core'))
2930
testImplementation(testArtifact(project(":libs:exponential-histogram")))
3031
yamlRestTestImplementation(testArtifact(project(xpackModule('core'))))

‎x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/CompressedExponentialHistogram.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.exponentialhistogram.CopyableBucketIterator;
1616
import org.elasticsearch.exponentialhistogram.ExponentialHistogram;
1717
import org.elasticsearch.exponentialhistogram.ZeroBucket;
18+
import org.elasticsearch.xpack.analytics.mapper.IndexWithCount;
1819

1920
import java.io.IOException;
2021
import java.util.List;

‎x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/EncodedHistogramData.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.common.io.stream.ByteArrayStreamInput;
1414
import org.elasticsearch.common.io.stream.BytesStreamOutput;
1515
import org.elasticsearch.common.io.stream.StreamOutput;
16+
import org.elasticsearch.xpack.analytics.mapper.IndexWithCount;
1617

1718
import java.io.IOException;
1819
import java.util.List;

0 commit comments

Comments
 (0)