Skip to content

Extend time range bucketing attributes to retrievers #136072

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 7, 2025
+344 −84
Merged

Extend time range bucketing attributes to retrievers #136072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c501e8a
Extend time range bucketing attributes to retrievers
javanna Oct 7, 2025
b3ad15c
Update docs/changelog/136072.yaml
javanna Oct 7, 2025
e26ad4f
Merge branch 'main' into enhancement/time_range_slicing_retrievers
javanna Oct 7, 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/136072.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 136072
summary: Extend time range bucketing attributes to retrievers
area: Search
type: enhancement
issues: []
43 changes: 42 additions & 1 deletion server/src/main/java/org/elasticsearch/action/search/SearchRequestAttributesExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,14 @@
import org.elasticsearch.index.query.NestedQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.RangeQueryBuilder;
import org.elasticsearch.index.query.RankDocsQueryBuilder;
import org.elasticsearch.search.SearchService;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.search.internal.ShardSearchRequest;
import org.elasticsearch.search.retriever.CompoundRetrieverBuilder;
import org.elasticsearch.search.retriever.KnnRetrieverBuilder;
import org.elasticsearch.search.retriever.RetrieverBuilder;
import org.elasticsearch.search.retriever.StandardRetrieverBuilder;
import org.elasticsearch.search.sort.FieldSortBuilder;
import org.elasticsearch.search.sort.ScoreSortBuilder;
import org.elasticsearch.search.sort.SortBuilder;
Expand Down Expand Up @@ -108,7 +113,15 @@ private static Map<String, Object> extractAttributes(
try {
introspectQueryBuilder(searchSourceBuilder.query(), queryMetadataBuilder, 0);
} catch (Exception e) {
logger.error("Failed to extract query attribute", e);
logger.error("Failed to extract query attributes", e);
}
}

if (searchSourceBuilder.retriever() != null) {
try {
introspectRetriever(searchSourceBuilder.retriever(), queryMetadataBuilder, 0);
} catch (Exception e) {
logger.error("Failed to extract retriever attributes", e);
}
}

Expand Down Expand Up @@ -311,6 +324,14 @@ private static void introspectQueryBuilder(QueryBuilder queryBuilder, QueryMetad
case NestedQueryBuilder nested:
introspectQueryBuilder(nested.query(), queryMetadataBuilder, ++level);
break;
case RankDocsQueryBuilder rankDocs:
QueryBuilder[] queryBuilders = rankDocs.getQueryBuilders();
if (queryBuilders != null) {
for (QueryBuilder builder : queryBuilders) {
introspectQueryBuilder(builder, queryMetadataBuilder, level + 1);
}
}
break;
case RangeQueryBuilder range:
// Note that the outcome of this switch differs depending on whether it is executed on the coord node, or data node.
// Data nodes perform query rewrite on each shard. That means that a query that reports a certain time range filter at the
Expand Down Expand Up @@ -338,6 +359,26 @@ private static void introspectQueryBuilder(QueryBuilder queryBuilder, QueryMetad
}
}

private static void introspectRetriever(RetrieverBuilder retrieverBuilder, QueryMetadataBuilder queryMetadataBuilder, int level) {
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 am not particularly happy about having to make this change. I think that retrievers with a time range filter are a bit of an edge case that is not so important for our current metrics effort around search performance.

That said, this change is required to avoid weird discrepancies due to the very general way that we track the time range filter from, without extensive knowledge of where we get it from. We could disable tracking it in certain situations as an alternative, but retrievers get rewritten to ordinary queries hence at the shard level we don't have the info to be able to make the distinction.

if (level > 20) {
return;
}
switch (retrieverBuilder) {
case KnnRetrieverBuilder knn:
queryMetadataBuilder.knnQuery = true;
break;
case StandardRetrieverBuilder standard:
introspectQueryBuilder(standard.topDocsQuery(), queryMetadataBuilder, level + 1);
break;
case CompoundRetrieverBuilder<?> compound:
for (CompoundRetrieverBuilder.RetrieverSource retrieverSource : compound.innerRetrievers()) {
introspectRetriever(retrieverSource.retriever(), queryMetadataBuilder, level + 1);
}
break;
default:
}
}

private enum TimeRangeBucket {
FifteenMinutes(TimeValue.timeValueMinutes(15).getMillis(), "15_minutes"),
OneHour(TimeValue.timeValueHours(1).getMillis(), "1_hour"),
Expand Down
4 changes: 4 additions & 0 deletions server/src/main/java/org/elasticsearch/index/query/RankDocsQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ protected void doWriteTo(StreamOutput out) throws IOException {
}
}

public QueryBuilder[] getQueryBuilders() {
return queryBuilders;
}

@Override
public String getWriteableName() {
return NAME;
Expand Down
Loading