Have top level knn searches tracked in query stats

[benwtrent]

Since dfs kNN searches aren't in the query phase, we don't get their search stats for free in query stats.

This adds their stats specifically during knn search in dfs.

closes: #128098

[elasticsearchmachine]

Pinging @elastic/es-search-relevance (Team:Search Relevance)

[elasticsearchmachine]

Hi @benwtrent, I've created a changelog YAML for you.

[carlosdelest]

The change LGTM 👍

A query with both knn section and a query will have multiple onPreQueryPhase / onQueryPhase calls. I wonder if we should have a different listener method for it, so we can tell apart the knn section from other queries?

[benwtrent]

@carlosdelest I went and checked the logic for the onPreQueryPhase

    public void onPreQueryPhase(SearchContext searchContext) {
        computeStats(
            searchContext,
            searchContext.hasOnlySuggest() ? statsHolder -> statsHolder.suggestCurrent.inc() : statsHolder -> statsHolder.queryCurrent.inc()
        );
    }

It looks like (for knn) it would only call statsHolder.queryCurrent.inc() which is then adjusted later in onQueryPhase to decrement the query being done.

However, I did notice that if there was a failure, we might not actually decrement this correctly as we don't catch and then call onFailedQueryPhase (which also decrements the counter)

I will add a try/finally here to make sure we decrement.

[carlosdelest]

I will add a try/finally here to make sure we decrement.

Looks good! 👍

[stures]

Sorry for waking this up, I see this is labeled 9.2.0, is this or will fix this be added to a minor fix of the 8.x major version? Or do we need to upgrade to 9 to get this fix?