[CI] ShardSearchPhaseAPMMetricsTests testUniformCanMatchMetricAttributesWhenPlentyOfDocumentsInIndex failed #140848
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tesWhenPlentyOfDocumentsInIndex failed - Unmute test and handle search request attributes that are not uniform for all shards.
elasticsearchmachine
added
the
needs:triage
chrisparrinello
added
Team:Search Foundations
Collaborator
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
elasticsearchmachine
added
the
needs:risk
chrisparrinello
added
low-risk
spinscale
approved these changes
Jan 20, 2026
| static final String KNN_ATTRIBUTE = "knn"; | ||
| static final String TIME_RANGE_FILTER_FIELD_ATTRIBUTE = "time_range_filter_field"; | ||
| static final String TIME_RANGE_FILTER_FROM_ATTRIBUTE = "time_range_filter_from"; | ||
| public static final String TARGET_ATTRIBUTE = "target"; |
Contributor
There was a problem hiding this comment.
these could stay package private?
Contributor
Author
There was a problem hiding this comment.
Actually I think I have an incomplete change here. I wanted to reuse TIME_RANGE_FILTER_FROM_ATTRIBUTE instead of the literal string in SearchService so I made that constant public and decided to just make them all public.
I'd like to definitely make TIME_RANGE_FILTER_FROM_ATTRIBUTE public and reuse that in SearchService. Should I keep the others still package private?
szybia
added a commit
to szybia/elasticsearch
that referenced
this pull request
Jan 21, 2026
…-tests * upstream/main: (104 commits) Partition time-series source (elastic#140475) Mute org.elasticsearch.xpack.esql.heap_attack.HeapAttackSubqueryIT testManyRandomKeywordFieldsInSubqueryIntermediateResultsWithSortManyFields elastic#141083 Reindex relocation: skip nodes marked for shutdown (elastic#141044) Make fails on fixture caching not fail image building (elastic#140959) Add multi-project tests for get and list reindex (elastic#140980) Painless docs overhaul (reference) (elastic#137211) Panama vector implementation of codePointCount (elastic#140693) Enable PromQL in release builds (elastic#140808) Update rest-api-spec for Jina embedding task (elastic#140696) [CI] ShardSearchPhaseAPMMetricsTests testUniformCanMatchMetricAttributesWhenPlentyOfDocumentsInIndex failed (elastic#140848) Combine hash computation with bloom filter writes/reads (elastic#140969) Refactor posting iterators to provide more information (elastic#141058) Wait for cluster to recover to yellow before checking index health (elastic#141057) (elastic#141065) Fix repo analysis read count assertions (elastic#140994) Fixed a bug in logsdb rolling upgrade sereverless tests involving par… (elastic#141022) Fix readiness edge case on startup (elastic#140791) PromQL: fix quantile function (elastic#141033) ignore `mmr` command for check (in development) (elastic#140981) Use Double.compare to compare doubles in tdigest.Sort (elastic#141049) Migrate third party module tests using legacy test clusters framework (elastic#140991) ...
jozala
reviewed
Jan 22, 2026
Comment on lines
+288
to
+290
| - class: org.elasticsearch.packaging.test.DockerTests | ||
| method: test072RunEsAsDifferentUserAndGroup | ||
| issue: https://github.com/elastic/elasticsearch/issues/140127 |
Contributor
There was a problem hiding this comment.
Hi @chrisparrinello,
Was this muted tests added here on purpose or it is conflict resolution mistake?
I'm looking through the DockerTests failures and the issue #140127 is closed but the mute is still here.
If it is a mistake I could take care of that as part of the work I'm doing around DockerTests.
Contributor
Author
|
@jozala This is a mistake. I think I had a bad merge from main that accidentally added it. Please feel free to remove that muted test. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #140589 and #140603
The underlying issue is that when gathering the search request attributes for the can-match APM metrics, we only do that for the first shard. The test is assuming that the search request attributes have all of the attributes including
time_range_filter_from. If the first search shard request is on an empty shard, we don't drill down into the query to determine if this is a time range query so thetime_range_filter_fromwill be missing from the search request attributes for all metrics for all of the shards.To fix this and ensure we're gathering as much search request attributes as possible for all search queries, we check each shard search request to see if there is a time request attribute in case it is missing in the cached search request attributes we've already parse from the first shard search request and then add it to the search request attributes.