Skip to content

Add shard_fetch_with_source_count metric#931

Open
shivamg2017 wants to merge 1 commit intoopensearch-project:mainfrom
shivamg2017:is-source-created
Open

Add shard_fetch_with_source_count metric#931
shivamg2017 wants to merge 1 commit intoopensearch-project:mainfrom
shivamg2017:is-source-created

Conversation

@shivamg2017
Copy link
Copy Markdown

Description

Add shard_fetch_with_source_count metric. This will emit a counter on fetch phase when source is requested.

ImmutableMetricData{resource=Resource{schemaUrl=null, attributes={service.name="OpenSearch"}}, instrumentationScopeInfo=InstrumentationScopeInfo{name=org.opensearch.telemetry, version=null, schemaUrl=null, attributes={}}, name=shard_fetch_with_source_count, description=Count of shard fetch phases where _source was requested, unit=count, type=DOUBLE_SUM, data=ImmutableSumData{points=[ImmutableDoublePointData{startEpochNanos=1774959167219102530, epochNanos=1774959227218847618, attributes={index_name="test-index", index_uuid="LlP0tKOESyi5Bq7IVi0sDw", shard_id=0}, value=1.0, exemplars=[]}], monotonic=true, aggregationTemporality=DELTA}}

try {
if (searchContext.fetchSourceContext() != null
&& searchContext.fetchSourceContext().fetchSource()) {
shardFetchWithSourceCounter.add(1, createTags(searchContext));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need shardId as dimension?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Removed — shard_id just adds cardinality without value.

return;
}
try {
if (searchContext.fetchSourceContext() != null
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can't we use context.sourceRequested()?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good call, updated.

private final Counter shardFetchWithSourceCounter;
private final int numProcessors;

public static final String SHARD_FETCH_WITH_SOURCE_COUNT = "shard_fetch_with_source_count";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we name it query_source_fetch_count?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, renamed.

1, createIndexTags(searchContext));
}
} catch (Exception e) {
LOG.debug("Error emitting query_source_fetch_count metric", e);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Print it as error log

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

private final Counter shardFetchWithSourceCounter;
private final int numProcessors;

public static final String QUERY_SOURCE_FETCH_COUNT = "query_source_fetch_count";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: let;s change to query_source_hits.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

@Gaganjuneja
Copy link
Copy Markdown
Collaborator

ADD UTs as well.

@shivamg2017
Copy link
Copy Markdown
Author

ADD UTs as well.

Added

@Gaganjuneja
Copy link
Copy Markdown
Collaborator

Please fix DCO.

Signed-off-by: Shivam Gupta <gshhivam@amazon.com>
return;
}
try {
if (searchContext.sourceRequested()) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is shard level correct so we will send counter as 1 for all shards. ?

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.

3 participants