Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
fad3c42
Add time range bucketing attribute to APM took time latency metrics
javanna Sep 26, 2025
e81fb6c
iter
javanna Sep 26, 2025
6470960
iter
javanna Sep 26, 2025
1aed53d
Update docs/changelog/135549.yaml
javanna Sep 26, 2025
6eb53a3
[CI] Auto commit changes from spotless
Sep 26, 2025
3967f72
iter
javanna Sep 26, 2025
3eb28fa
iter
javanna Sep 26, 2025
d9fdc5b
iter
javanna Sep 26, 2025
cc195d4
[CI] Auto commit changes from spotless
Sep 26, 2025
b5f144d
iter
javanna Sep 26, 2025
d81c985
Merge branch 'main' into enhancement/took_time_time_range_bucketing
javanna Sep 26, 2025
532eb96
iter
javanna Sep 26, 2025
0684e86
Merge branch 'main' into enhancement/took_time_time_range_bucketing
javanna Sep 28, 2025
ff41cb0
Update server/src/main/java/org/elasticsearch/search/query/QueryPhase…
javanna Sep 29, 2025
f985361
Update server/src/main/java/org/elasticsearch/search/query/QuerySearc…
javanna Sep 29, 2025
2ba5023
iter
javanna Sep 29, 2025
bcffd12
Merge branch 'main' into enhancement/took_time_time_range_bucketing
javanna Sep 29, 2025
d663723
iter
javanna Sep 29, 2025
5eaf6db
iter
javanna Sep 29, 2025
233db24
[CI] Auto commit changes from spotless
Sep 29, 2025
ba23911
Merge branch 'main' into enhancement/took_time_time_range_bucketing
javanna Sep 29, 2025
53a80d0
iter
javanna Sep 29, 2025
54a8a4f
[CI] Auto commit changes from spotless
Sep 29, 2025
97cb335
[CI] Update transport version definitions
Sep 29, 2025
3f47f9a
iter
javanna Sep 30, 2025
6948a16
Merge branch 'main' into enhancement/took_time_time_range_bucketing
javanna Sep 30, 2025
e28898f
iter
javanna Sep 30, 2025
edffb03
Merge branch 'main' into enhancement/took_time_time_range_bucketing
javanna Oct 1, 2025
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
5 changes: 5 additions & 0 deletions docs/changelog/135549.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 135549
summary: Add time range bucketing attribute to APM took time latency metrics
area: Search
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ public SearchPhaseController.ReducedQueryPhase reduce() throws Exception {
1,
0,
0,
results.isEmpty()
results.isEmpty(),
null
);
if (progressListener != SearchProgressListener.NOOP) {
progressListener.notifyFinalReduce(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,8 @@ private SearchPhaseController.ReducedQueryPhase newReducedQueryPhaseResults(
reducedQueryPhase.numReducePhases(),
reducedQueryPhase.size(),
reducedQueryPhase.from(),
reducedQueryPhase.isEmptyResult()
reducedQueryPhase.isEmptyResult(),
reducedQueryPhase.rangeTimestampFrom()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,8 @@ static ReducedQueryPhase reducedQueryPhase(
numReducePhases,
0,
0,
true
true,
null
);
}
final List<QuerySearchResult> nonNullResults = new ArrayList<>();
Expand Down Expand Up @@ -516,6 +517,7 @@ static ReducedQueryPhase reducedQueryPhase(
: Collections.emptyMap();
int from = 0;
int size = 0;
Long rangeTimestampFrom = null;
DocValueFormat[] sortValueFormats = null;
for (QuerySearchResult result : nonNullResults) {
from = result.from();
Expand All @@ -525,6 +527,11 @@ static ReducedQueryPhase reducedQueryPhase(
sortValueFormats = result.sortValueFormats();
}

if (rangeTimestampFrom == null) {
// we simply take the first one: we should get the same value from all shards anyways
rangeTimestampFrom = result.getRangeTimestampFrom();
}

if (hasSuggest) {
assert result.suggest() != null;
for (Suggestion<? extends Suggestion.Entry<? extends Suggestion.Entry.Option>> suggestion : result.suggest()) {
Expand Down Expand Up @@ -579,7 +586,8 @@ static ReducedQueryPhase reducedQueryPhase(
numReducePhases,
size,
from,
false
false,
rangeTimestampFrom
);
}

Expand Down Expand Up @@ -662,7 +670,8 @@ public record ReducedQueryPhase(
// the offset into the merged top hits
int from,
// <code>true</code> iff the query phase had no results. Otherwise <code>false</code>
boolean isEmptyResult
boolean isEmptyResult,
Long rangeTimestampFrom
) {

public ReducedQueryPhase {
Expand All @@ -683,7 +692,8 @@ public SearchResponseSections buildResponse(SearchHits hits, Collection<? extend
timedOut,
terminatedEarly,
buildSearchProfileResults(fetchResults),
numReducePhases
numReducePhases,
rangeTimestampFrom
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,13 @@ private enum TimeRangeBucket {
}
}

public static void addTimeRangeAttribute(Long timeRangeFrom, long nowInMillis, Map<String, Object> attributes) {
if (timeRangeFrom != null) {
String timestampRangeFilter = introspectTimeRange(timeRangeFrom, nowInMillis);
attributes.put(TIMESTAMP_RANGE_FILTER_ATTRIBUTE, timestampRangeFilter);
}
}

static String introspectTimeRange(long timeRangeFrom, long nowInMillis) {
for (TimeRangeBucket value : TimeRangeBucket.values()) {
if (timeRangeFrom >= nowInMillis - value.millis) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ public class SearchResponse extends ActionResponse implements ChunkedToXContentO
private final ShardSearchFailure[] shardFailures;
private final Clusters clusters;
private final long tookInMillis;
// only used for telemetry purposes on the coordinating node, where the search response gets created
private transient Long rangeTimestampFrom;

private final RefCounted refCounted = LeakTracker.wrap(new SimpleRefCounted());

Expand Down Expand Up @@ -187,6 +189,7 @@ public SearchResponse(
clusters,
pointInTimeId
);
this.rangeTimestampFrom = searchResponseSections.rangeTimestampFrom;
}

public SearchResponse(
Expand Down Expand Up @@ -464,6 +467,10 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalBytesReference(pointInTimeId);
}

public Long getRangeTimestampFrom() {
return rangeTimestampFrom;
}

@Override
public String toString() {
return hasReferences() == false ? "SearchResponse[released]" : Strings.toString(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ public class SearchResponseSections implements Releasable {
false,
null,
null,
1
1,
null
);
public static final SearchResponseSections EMPTY_WITHOUT_TOTAL_HITS = new SearchResponseSections(
SearchHits.EMPTY_WITHOUT_TOTAL_HITS,
Expand All @@ -41,7 +42,8 @@ public class SearchResponseSections implements Releasable {
false,
null,
null,
1
1,
null
);
protected final SearchHits hits;
protected final InternalAggregations aggregations;
Expand All @@ -50,6 +52,7 @@ public class SearchResponseSections implements Releasable {
protected final boolean timedOut;
protected final Boolean terminatedEarly;
protected final int numReducePhases;
protected final Long rangeTimestampFrom;

public SearchResponseSections(
SearchHits hits,
Expand All @@ -58,7 +61,8 @@ public SearchResponseSections(
boolean timedOut,
Boolean terminatedEarly,
SearchProfileResults profileResults,
int numReducePhases
int numReducePhases,
Long rangeTimestampFrom
) {
this.hits = hits;
this.aggregations = aggregations;
Expand All @@ -67,6 +71,7 @@ public SearchResponseSections(
this.timedOut = timedOut;
this.terminatedEarly = terminatedEarly;
this.numReducePhases = numReducePhases;
this.rangeTimestampFrom = rangeTimestampFrom;
}

public final SearchHits hits() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,12 @@ public void onFailure(Exception e) {
Arrays.stream(resolvedIndices.getConcreteLocalIndices()).map(Index::getName).toArray(String[]::new)
);
if (collectCCSTelemetry == false || resolvedIndices.getRemoteClusterIndices().isEmpty()) {
searchResponseActionListener = new SearchTelemetryListener(delegate, searchResponseMetrics, searchRequestAttributes);
searchResponseActionListener = new SearchTelemetryListener(
delegate,
searchResponseMetrics,
searchRequestAttributes,
timeProvider.absoluteStartMillis()
);
} else {
CCSUsage.Builder usageBuilder = new CCSUsage.Builder();
usageBuilder.setRemotesCount(resolvedIndices.getRemoteClusterIndices().size());
Expand Down Expand Up @@ -459,6 +464,7 @@ public void onFailure(Exception e) {
delegate,
searchResponseMetrics,
searchRequestAttributes,
timeProvider.absoluteStartMillis(),
usageService,
usageBuilder
);
Expand Down Expand Up @@ -2046,6 +2052,7 @@ static String[] ignoreBlockedIndices(ProjectState projectState, String[] concret
private static class SearchTelemetryListener extends DelegatingActionListener<SearchResponse, SearchResponse> {
private final CCSUsage.Builder usageBuilder;
private final SearchResponseMetrics searchResponseMetrics;
private final long nowInMillis;
private final UsageService usageService;
private final boolean collectCCSTelemetry;
private final Map<String, Object> searchRequestAttributes;
Expand All @@ -2054,12 +2061,14 @@ private static class SearchTelemetryListener extends DelegatingActionListener<Se
ActionListener<SearchResponse> listener,
SearchResponseMetrics searchResponseMetrics,
Map<String, Object> searchRequestAttributes,
long nowInMillis,
UsageService usageService,
CCSUsage.Builder usageBuilder
) {
super(listener);
this.searchResponseMetrics = searchResponseMetrics;
this.searchRequestAttributes = searchRequestAttributes;
this.nowInMillis = nowInMillis;
this.collectCCSTelemetry = true;
this.usageService = usageService;
this.usageBuilder = usageBuilder;
Expand All @@ -2068,11 +2077,13 @@ private static class SearchTelemetryListener extends DelegatingActionListener<Se
SearchTelemetryListener(
ActionListener<SearchResponse> listener,
SearchResponseMetrics searchResponseMetrics,
Map<String, Object> searchRequestAttributes
Map<String, Object> searchRequestAttributes,
long nowInMillis
) {
super(listener);
this.searchResponseMetrics = searchResponseMetrics;
this.searchRequestAttributes = searchRequestAttributes;
this.nowInMillis = nowInMillis;
this.collectCCSTelemetry = false;
this.usageService = null;
this.usageBuilder = null;
Expand All @@ -2081,7 +2092,12 @@ private static class SearchTelemetryListener extends DelegatingActionListener<Se
@Override
public void onResponse(SearchResponse searchResponse) {
try {
searchResponseMetrics.recordTookTime(searchResponse.getTookInMillis(), searchRequestAttributes);
searchResponseMetrics.recordTookTime(
searchResponse.getTookInMillis(),
searchResponse.getRangeTimestampFrom(),
nowInMillis,
searchRequestAttributes
);
SearchResponseMetrics.ResponseCountTotalStatus responseCountTotalStatus =
SearchResponseMetrics.ResponseCountTotalStatus.SUCCESS;
if (searchResponse.getShardFailures() != null && searchResponse.getShardFailures().length > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ public Relation isFieldWithinQuery(
minValue = Long.min(minValue, skipper.minValue());
maxValue = Long.max(maxValue, skipper.maxValue());
}
return isFieldWithinQuery(minValue, maxValue, from, to, includeLower, includeUpper, timeZone, dateParser, context);
return isFieldWithinQuery(minValue, maxValue, from, to, includeLower, includeUpper, timeZone, dateParser, context, name());
}
byte[] minPackedValue = PointValues.getMinPackedValue(reader, name());
if (minPackedValue == null) {
Expand All @@ -954,7 +954,7 @@ public Relation isFieldWithinQuery(
long minValue = LongPoint.decodeDimension(minPackedValue, 0);
long maxValue = LongPoint.decodeDimension(PointValues.getMaxPackedValue(reader, name()), 0);

return isFieldWithinQuery(minValue, maxValue, from, to, includeLower, includeUpper, timeZone, dateParser, context);
return isFieldWithinQuery(minValue, maxValue, from, to, includeLower, includeUpper, timeZone, dateParser, context, name());
}

public DateMathParser resolveDateMathParser(DateMathParser dateParser, Object from, Object to) {
Expand All @@ -977,7 +977,8 @@ public Relation isFieldWithinQuery(
boolean includeUpper,
ZoneId timeZone,
DateMathParser dateParser,
QueryRewriteContext context
QueryRewriteContext context,
String fieldName
) {
dateParser = resolveDateMathParser(dateParser, from, to);

Expand All @@ -990,6 +991,9 @@ public Relation isFieldWithinQuery(
}
++fromInclusive;
}
if (fieldName.equals(DataStream.TIMESTAMP_FIELD_NAME)) {
context.setRangeTimestampFrom(fromInclusive);
}
}

long toInclusive = Long.MAX_VALUE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,13 @@ public String getWriteableName() {
protected Query doToQuery(SearchExecutionContext context) throws IOException {
BooleanQuery.Builder booleanQueryBuilder = new BooleanQuery.Builder();
addBooleanClauses(context, booleanQueryBuilder, mustClauses, BooleanClause.Occur.MUST);
addBooleanClauses(context, booleanQueryBuilder, mustNotClauses, BooleanClause.Occur.MUST_NOT);
addBooleanClauses(context, booleanQueryBuilder, shouldClauses, BooleanClause.Occur.SHOULD);
try {
context.setTrackRangeTimestampFrom(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

should we document why we disable this tracking here?

maybe secondary, but if we have only one should clause, we want to track the range?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can add a comment, yes we could have a special case for bool with a single should.

Copy link
Member Author

Choose a reason for hiding this comment

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

this reminds me of the current discrepancy between the boolean attribute that signals whether there is a filter on timestamp, and the range bucket. I meant to ask you what you think about this.

If a shard has all documents within the range, it is in fact not a range query when it comes to the lucene level, but rather a match_all. I thought that we'd want to track this distinction.

At the coord level, we will always extract the original range before query rewrite and set to flag to true.
At the shard level, only those shards that effectively execute a range query will have the boolean flag set to true, but all will have the range extracted. Would we want instead the two to be consistent? I even challenged at some point that we still want the query introspection if all we use it for is to figure out whether there was a filter on @timestamp: we now have another way to do that which does not require visiting the query tree, and perhaps we don't even need the boolean flag anymore.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking further, I don't think I will add a special case for bool with a single should clause. I already don't like how intrusive the metrics tracking is in the actual code that does stuff. I am way of adding more logic to control when to report on metrics, intermingled with the actual code that rewrites and executes queries. I think that having a time range as a single should clause it also quite an edge case. If needed we can always optimize it later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, thanks for looking into this

addBooleanClauses(context, booleanQueryBuilder, mustNotClauses, BooleanClause.Occur.MUST_NOT);
addBooleanClauses(context, booleanQueryBuilder, shouldClauses, BooleanClause.Occur.SHOULD);
} finally {
context.setTrackRangeTimestampFrom(true);
}
addBooleanClauses(context, booleanQueryBuilder, filterClauses, BooleanClause.Occur.FILTER);
BooleanQuery booleanQuery = booleanQueryBuilder.build();
if (booleanQuery.clauses().isEmpty()) {
Expand Down Expand Up @@ -348,9 +353,21 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws
return new MatchAllQueryBuilder().boost(boost()).queryName(queryName());
}
changed |= rewriteClauses(queryRewriteContext, mustClauses, newBuilder::must);
changed |= rewriteClauses(queryRewriteContext, mustNotClauses, newBuilder::mustNot);

try {
queryRewriteContext.setTrackRangeTimestampFrom(false);
changed |= rewriteClauses(queryRewriteContext, mustNotClauses, newBuilder::mustNot);
} finally {
queryRewriteContext.setTrackRangeTimestampFrom(true);
}
changed |= rewriteClauses(queryRewriteContext, filterClauses, newBuilder::filter);
changed |= rewriteClauses(queryRewriteContext, shouldClauses, newBuilder::should);
try {
queryRewriteContext.setTrackRangeTimestampFrom(false);
changed |= rewriteClauses(queryRewriteContext, shouldClauses, newBuilder::should);
} finally {
queryRewriteContext.setTrackRangeTimestampFrom(true);
}

// early termination when must clause is empty and optional clauses is returning MatchNoneQueryBuilder
if (mustClauses.size() == 0
&& filterClauses.size() == 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ public class QueryRewriteContext {
private QueryRewriteInterceptor queryRewriteInterceptor;
private final Boolean ccsMinimizeRoundTrips;
private final boolean isExplain;
private Long rangeTimestampFrom;
private boolean trackRangeTimestampFrom = true;

public QueryRewriteContext(
final XContentParserConfiguration parserConfiguration,
Expand Down Expand Up @@ -520,4 +522,32 @@ public void setQueryRewriteInterceptor(QueryRewriteInterceptor queryRewriteInter
this.queryRewriteInterceptor = queryRewriteInterceptor;
}

/**
* Returns the minimum lower bound across the time ranges filters against the @timestamp field included in the query
*/
public Long getRangeTimestampFrom() {
return rangeTimestampFrom;
}

/**
* Records the lower bound of a time range filter against the @timestamp field included in the query. For telemetry purposes.
*/
public void setRangeTimestampFrom(long rangeTimestampFrom) {
if (trackRangeTimestampFrom) {
if (this.rangeTimestampFrom == null) {
this.rangeTimestampFrom = rangeTimestampFrom;
} else {
this.rangeTimestampFrom = Math.min(rangeTimestampFrom, this.rangeTimestampFrom);
}
}
}

/**
* Enables or disables the tracking of the lower bound for time range filters against the @timestamp field,
* done via {@link #setRangeTimestampFrom(long)}. Tracking is enabled by default, and explicitly disabled to ensure
* we don't record the bound for range queries within should and must_not clauses.
*/
public void setTrackRangeTimestampFrom(boolean trackRangeTimestampFrom) {
this.trackRangeTimestampFrom = trackRangeTimestampFrom;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,8 @@ protected MappedFieldType.Relation getRelation(final CoordinatorRewriteContext c
includeUpper,
timeZone,
dateMathParser,
coordinatorRewriteContext
coordinatorRewriteContext,
dateFieldType.name()
);
}
// If the field type is null or not of type DataFieldType then we have no idea whether this range query will match during
Expand Down
Loading