Skip to content

Add search document count metric#580

Open
shivamg2017 wants to merge 1 commit intoopensearch-project:mainfrom
shivamg2017:search_doc_metric
Open

Add search document count metric#580
shivamg2017 wants to merge 1 commit intoopensearch-project:mainfrom
shivamg2017:search_doc_metric

Conversation

@shivamg2017
Copy link
Copy Markdown

Description

Add search document count metric
Metric to fetch doc count returned by search query.

Relevant Opensearch PR to update SearchRequestContext : opensearch-project/OpenSearch#21037

metric: ImmutableMetricData{resource=Resource{schemaUrl=null, attributes={service.name="OpenSearch"}}, instrumentationScopeInfo=InstrumentationScopeInfo{name=org.opensearch.telemetry, version=null, schemaUrl=null, attributes={}}, name=search.query.doc_count.histogram, description=Histogram for the number of documents returned per search query, unit=1, type=EXPONENTIAL_HISTOGRAM, data=ImmutableExponentialHistogramData{aggregationTemporality=CUMULATIVE, points=[ImmutableExponentialHistogramPointData{getStartEpochNanos=1774810013641740000, getEpochNanos=1774810493652552000, getAttributes={}, getScale=20, getSum=12.0, getCount=4, getZeroCount=0, hasMin=true, getMin=3.0, hasMax=true, getMax=3.0, getPositiveBuckets=DoubleExponentialHistogramBuckets{scale: 20, offset: 1661953, counts: {1661953=4} }, getNegativeBuckets=EmptyExponentialHistogramBuckets{scale=20, offset=0, bucketCounts=[], totalCount=0}, getExemplars=[]}]}}

Signed-off-by: shivamg2017 <shivamgupta.6july@gmail.com>
Copy link
Copy Markdown
Member

@dzane17 dzane17 left a comment

Choose a reason for hiding this comment

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

Can you add tests?
QueryInsightsListenerTests.testOnRequestEnd should mock searchRequestContext.getHitsLength() and assert the value on the captured record, similar to how isStreamingRequest() / isStreaming() is tested today.
SearchQueryCategorizerTests should verify that recordSearchDocCount is called with the expected value when categorize runs.

*
* @param other the {@link SearchQueryRecord} to copy.
*/
public SearchQueryRecord(SearchQueryRecord other) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add hitsLength to the copy constructor. I know the value is only used for categorization histograms, but we should still keep this method up to date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants