Skip to content

Can match phase shard APM metric with search request context#137196

Merged
chrisparrinello merged 2 commits intoelastic:mainfrom
chrisparrinello:can_match_phase_shard_metric_with_context
Nov 12, 2025
Merged

Can match phase shard APM metric with search request context#137196
chrisparrinello merged 2 commits intoelastic:mainfrom
chrisparrinello:can_match_phase_shard_metric_with_context

Conversation

@chrisparrinello
Copy link
Contributor

@chrisparrinello chrisparrinello commented Oct 27, 2025 •
edited
Loading

Add search request attributes context to:

  • es.search.shards.phases.can_match.duration.histogram
@elasticsearchmachine elasticsearchmachine added v9.3.0 needs:triage Requires assignment of a team area label labels Oct 27, 2025
@chrisparrinello chrisparrinello changed the title Can match phase shard metric with context Oct 27, 2025
@chrisparrinello chrisparrinello added >enhancement Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch :Search Foundations/Search Catch all for Search Foundations and removed needs:triage Requires assignment of a team area label labels Oct 27, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Oct 27, 2025

Pinging @elastic/es-search-foundations (Team:Search Foundations)
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Oct 27, 2025

Hi @chrisparrinello, I've created a changelog YAML for you.
javanna
javanna reviewed Oct 28, 2025
View reviewed changes
server/src/main/java/org/elasticsearch/search/SearchService.java
// TODO remove the exception handling as it's now in canMatch itself
responses.add(new CanMatchNodeResponse.ResponseOrFailure(canMatch(shardSearchRequest)));
indexShard.getSearchOperationListener().onCanMatchPhase(System.nanoTime() - shardCanMatchStartTimeInNanos);
} catch (Exception e) {
Copy link
Contributor

@javanna javanna Oct 28, 2025

Choose a reason for hiding this comment

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

Could you restore the try catch please and the TODO? Sadly, more work is required to remove it, around removing dead code from the can match serialization layer, add a transport version for it etc. Feel free to take that on as a follow-up if you wish, I can share more details about it.
Copy link
Contributor Author

@chrisparrinello chrisparrinello Oct 28, 2025

Choose a reason for hiding this comment

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

Will do!
Copy link
Contributor

@drempapis drempapis Nov 11, 2025

Choose a reason for hiding this comment

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

Do we still need to restore the TODO before responses.add(...)

// TODO remove the exception handling as it's now in canMatch itself
Copy link
Contributor Author

@chrisparrinello chrisparrinello Nov 11, 2025

Choose a reason for hiding this comment

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

It is restored based on looking at the full diff between main and this branch (actually the full diff shows the TODO as unmodified and in the try block, not the catch block). Sorry the commit history messed up when I did a bad rebase.
@javanna javanna mentioned this pull request Oct 29, 2025
Merged
javanna
javanna reviewed Oct 29, 2025
View reviewed changes
javanna added a commit that referenced this pull request Oct 30, 2025
@javanna
Refactor canMatch execution to expose time range filter to caller (#1…
9aed5f4 
…37314)

We recently introduced metrics attributes to track search latencies at the
shard and coord level.

With #137196 we are introducing such attributes to the can match phase latency
metrics. The time range filter is not currently accessible when recording the metrics. 
This commit exposes it.
@chrisparrinello chrisparrinello requested review from a team as code owners November 3, 2025 20:11
chrisparrinello pushed a commit to chrisparrinello/elasticsearch that referenced this pull request Nov 3, 2025
@javanna @chrisparrinello
Refactor canMatch execution to expose time range filter to caller (el…
a78e447 
…astic#137314)

We recently introduced metrics attributes to track search latencies at the
shard and coord level.

With elastic#137196 we are introducing such attributes to the can match phase latency
metrics. The time range filter is not currently accessible when recording the metrics.
This commit exposes it.
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025 •
edited
Loading

🔍 Preview links for changed docs

More links …
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

ℹ️ Important: Docs version tagging

👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version.

We use applies_to tags to mark version-specific features and changes.

Expand for a quick overview

When to use applies_to tags:

✅ At the page level to indicate which products/deployments the content applies to (mandatory)
✅ When features change state (e.g. preview, ga) in a specific version
✅ When availability differs across deployments and environments

What NOT to do:

❌ Don't remove or replace information that applies to an older version
❌ Don't add new information that applies to a specific version without an applies_to tag
❌ Don't forget that applies_to tags can be used at the page, section, and inline level

🤔 Need help?
@chrisparrinello chrisparrinello force-pushed the can_match_phase_shard_metric_with_context branch from eb689a2 to 40a500e Compare November 3, 2025 20:36
drempapis
drempapis approved these changes Nov 12, 2025
View reviewed changes
Copy link
Contributor

@drempapis drempapis left a comment

Choose a reason for hiding this comment

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

LGTM!
@chrisparrinello chrisparrinello merged commit 1d0f434 into elastic:main Nov 12, 2025
34 checks passed
@chrisparrinello chrisparrinello deleted the can_match_phase_shard_metric_with_context branch November 12, 2025 15:29
@benchaplin benchaplin mentioned this pull request Nov 12, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>enhancement :Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch v9.3.0

4 participants

@chrisparrinello @elasticsearchmachine @javanna @drempapis