-
Notifications
You must be signed in to change notification settings - Fork 25.5k
Add time range bucketing attribute to APM shard search latency metrics #135524
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
pr: 135524 | ||
summary: Add time range bucketing attribute to APM shard search latency metrics | ||
area: Search | ||
type: enhancement | ||
issues: [] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ | |
import org.apache.lucene.search.IndexSortSortedNumericDocValuesRangeQuery; | ||
import org.apache.lucene.search.Query; | ||
import org.elasticsearch.ElasticsearchParseException; | ||
import org.elasticsearch.cluster.metadata.DataStream; | ||
import org.elasticsearch.common.geo.ShapeRelation; | ||
import org.elasticsearch.common.logging.DeprecationCategory; | ||
import org.elasticsearch.common.logging.DeprecationLogger; | ||
|
@@ -747,33 +748,34 @@ public Query rangeQuery( | |
if (relation == ShapeRelation.DISJOINT) { | ||
throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] does not support DISJOINT ranges"); | ||
} | ||
DateMathParser parser; | ||
if (forcedDateParser == null) { | ||
if (lowerTerm instanceof Number || upperTerm instanceof Number) { | ||
// force epoch_millis | ||
parser = EPOCH_MILLIS_PARSER; | ||
} else { | ||
parser = dateMathParser; | ||
} | ||
} else { | ||
parser = forcedDateParser; | ||
} | ||
return dateRangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, timeZone, parser, context, resolution, (l, u) -> { | ||
Query query; | ||
if (isIndexed()) { | ||
query = LongPoint.newRangeQuery(name(), l, u); | ||
if (hasDocValues()) { | ||
Query dvQuery = SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u); | ||
query = new IndexOrDocValuesQuery(query, dvQuery); | ||
DateMathParser parser = resolveDateMathParser(forcedDateParser, lowerTerm, upperTerm); | ||
return dateRangeQuery( | ||
lowerTerm, | ||
upperTerm, | ||
includeLower, | ||
includeUpper, | ||
timeZone, | ||
parser, | ||
context, | ||
resolution, | ||
name(), | ||
(l, u) -> { | ||
Query query; | ||
if (isIndexed()) { | ||
query = LongPoint.newRangeQuery(name(), l, u); | ||
if (hasDocValues()) { | ||
Query dvQuery = SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u); | ||
query = new IndexOrDocValuesQuery(query, dvQuery); | ||
} | ||
} else { | ||
query = SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u); | ||
} | ||
} else { | ||
query = SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u); | ||
} | ||
if (hasDocValues() && context.indexSortedOnField(name())) { | ||
query = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, query); | ||
if (hasDocValues() && context.indexSortedOnField(name())) { | ||
query = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, query); | ||
} | ||
return query; | ||
} | ||
return query; | ||
}); | ||
); | ||
} | ||
|
||
public static Query dateRangeQuery( | ||
|
@@ -785,6 +787,7 @@ public static Query dateRangeQuery( | |
DateMathParser parser, | ||
SearchExecutionContext context, | ||
Resolution resolution, | ||
String fieldName, | ||
BiFunction<Long, Long, Query> builder | ||
) { | ||
return handleNow(context, nowSupplier -> { | ||
|
@@ -796,6 +799,9 @@ public static Query dateRangeQuery( | |
if (includeLower == false) { | ||
++l; | ||
} | ||
if (fieldName.equals(DataStream.TIMESTAMP_FIELD_NAME)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the core of the change: as we parse the lower bound of a time range, if it is against the |
||
context.setRangeTimestampFrom(l); | ||
} | ||
} | ||
if (upperTerm == null) { | ||
u = Long.MAX_VALUE; | ||
|
@@ -951,6 +957,17 @@ public Relation isFieldWithinQuery( | |
return isFieldWithinQuery(minValue, maxValue, from, to, includeLower, includeUpper, timeZone, dateParser, context); | ||
} | ||
|
||
public DateMathParser resolveDateMathParser(DateMathParser dateParser, Object from, Object to) { | ||
if (dateParser == null) { | ||
if (from instanceof Number || to instanceof Number) { | ||
// force epoch_millis | ||
return EPOCH_MILLIS_PARSER; | ||
} | ||
return this.dateMathParser; | ||
} | ||
return dateParser; | ||
} | ||
|
||
public Relation isFieldWithinQuery( | ||
long minValue, | ||
long maxValue, | ||
|
@@ -962,14 +979,7 @@ public Relation isFieldWithinQuery( | |
DateMathParser dateParser, | ||
QueryRewriteContext context | ||
) { | ||
if (dateParser == null) { | ||
if (from instanceof Number || to instanceof Number) { | ||
// force epoch_millis | ||
dateParser = EPOCH_MILLIS_PARSER; | ||
} else { | ||
dateParser = this.dateMathParser; | ||
} | ||
} | ||
dateParser = resolveDateMathParser(dateParser, from, to); | ||
|
||
long fromInclusive = Long.MIN_VALUE; | ||
if (from != null) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -361,4 +361,56 @@ public void testDepthLimit() { | |
assertAttributes(stringObjectMap, "user", "_score", "hits_only", false, false, false, null); | ||
} | ||
} | ||
|
||
public void testIntrospectTimeRange() { | ||
long nowInMillis = System.currentTimeMillis(); | ||
assertEquals("15_minutes", SearchRequestAttributesExtractor.introspectTimeRange(nowInMillis, nowInMillis)); | ||
|
||
long fifteenMinutesAgo = nowInMillis - (15 * 60 * 1000); | ||
assertEquals( | ||
"15_minutes", | ||
SearchRequestAttributesExtractor.introspectTimeRange(randomLongBetween(fifteenMinutesAgo, nowInMillis), nowInMillis) | ||
); | ||
|
||
long oneHourAgo = nowInMillis - (60 * 60 * 1000); | ||
assertEquals( | ||
"1_hour", | ||
SearchRequestAttributesExtractor.introspectTimeRange(randomLongBetween(oneHourAgo, fifteenMinutesAgo), nowInMillis) | ||
); | ||
|
||
long twelveHoursAgo = nowInMillis - (12 * 60 * 60 * 1000); | ||
assertEquals( | ||
"12_hours", | ||
SearchRequestAttributesExtractor.introspectTimeRange(randomLongBetween(twelveHoursAgo, oneHourAgo), nowInMillis) | ||
); | ||
|
||
long oneDayAgo = nowInMillis - (24 * 60 * 60 * 1000); | ||
assertEquals( | ||
"1_day", | ||
SearchRequestAttributesExtractor.introspectTimeRange(randomLongBetween(oneDayAgo, twelveHoursAgo), nowInMillis) | ||
); | ||
|
||
long threeDaysAgo = nowInMillis - (3 * 24 * 60 * 60 * 1000); | ||
assertEquals( | ||
"3_days", | ||
SearchRequestAttributesExtractor.introspectTimeRange(randomLongBetween(threeDaysAgo, oneDayAgo), nowInMillis) | ||
); | ||
|
||
long sevenDaysAgo = nowInMillis - (7 * 24 * 60 * 60 * 1000); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: could use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey, i tried to stay away from that because otherwise it is a 1:1 copy of the code it's testing. Maybe not so important, but I'd prefer keeping the test super simple. |
||
assertEquals( | ||
"7_days", | ||
SearchRequestAttributesExtractor.introspectTimeRange(randomLongBetween(sevenDaysAgo, threeDaysAgo), nowInMillis) | ||
); | ||
|
||
long fourteenDaysAgo = nowInMillis - (14 * 24 * 60 * 60 * 1000); | ||
assertEquals( | ||
"14_days", | ||
SearchRequestAttributesExtractor.introspectTimeRange(randomLongBetween(fourteenDaysAgo, sevenDaysAgo), nowInMillis) | ||
); | ||
|
||
assertEquals( | ||
"older_than_14_days", | ||
SearchRequestAttributesExtractor.introspectTimeRange(randomLongBetween(0, fourteenDaysAgo), nowInMillis) | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just addressing some duplication on parsing the time ranges that I spotted while i was working on it, no functional change