Add metric to count queries with script#923
Add metric to count queries with script#923shivamg2017 wants to merge 1 commit intoopensearch-project:mainfrom
Conversation
shivamg2017
requested review from
Gaganjuneja,
Shephalimittal,
ansjcy,
khushbr,
nishchay21,
psychbot,
rramachand21 and
sbcd90
as code owners
| "Count of search requests tagged by whether they contain a script", | ||
| RTFMetrics.MetricUnits.COUNT.toString()); | ||
| metricsInitialized = true; | ||
| System.out.println("PA_DEBUG: script_search_count counter initialized"); |
| if (!isEnabled()) { | ||
| return; | ||
| } | ||
| MetricsRegistry metricsRegistry = OpenSearchResources.INSTANCE.getMetricsRegistry(); |
There was a problem hiding this comment.
Better get this in the constructer itself?
There was a problem hiding this comment.
MetricsRegistry is null at construction time because this collector is instantiated inside getActionFilters(), which OpenSearch calls before createComponents() where the registry is injected. Its similar to https://github.com/opensearch-project/performance-analyzer/blob/main/src/main/java/org/opensearch/performanceanalyzer/ShardMetricsCollector.java#L62
...rg/opensearch/performanceanalyzer/collectors/telemetry/RTFSearchRequestMetricsCollector.java
Outdated
Show resolved
Hide resolved
| ActionListener<Response> listener, | ||
| ActionFilterChain<Request, Response> chain) { | ||
|
|
||
| if (controller.isPerformanceAnalyzerEnabled() |
There was a problem hiding this comment.
This code only works for RCA and DUAL mode. So your code changes will not run for RTF mode. Better write separate ActionFilter.
There was a problem hiding this comment.
Added below for RTF mode as well.
| try { | ||
| SearchSourceBuilder source = searchRequest.source(); | ||
| String sourceString = source.toString(); | ||
| return sourceString.contains("\"script\"") |
There was a problem hiding this comment.
How heavy is this operation?
There was a problem hiding this comment.
Remove string serialization. Now its O(1) operation
shivamg2017
force-pushed
the
script-metric
branch
from
a010f7b to
a230eee
Compare
| } | ||
| } | ||
|
|
||
| if (request instanceof SearchRequest) { |
There was a problem hiding this comment.
Let's create a separate ActionFilter for RTF and avoid touching this one.
| if (!isEnabled()) { | ||
| return; | ||
| } | ||
| MetricsRegistry metricsRegistry = OpenSearchResources.INSTANCE.getMetricsRegistry(); |
| * - Bucket script / bucket selector pipeline aggregations | ||
| * - Scripts inside sub-aggregations | ||
| */ | ||
| private boolean detectScript(SearchRequest searchRequest) { |
There was a problem hiding this comment.
Just wondering, Can't we publish this metric write creating the source in core?
| * - Bucket script / bucket selector pipeline aggregations | ||
| * - Scripts inside sub-aggregations | ||
| */ | ||
| private boolean detectScript(SearchRequest searchRequest) { |
There was a problem hiding this comment.
Follow naming conventions pls.
|
|
||
| return false; | ||
| } catch (Exception e) { | ||
| LOG.debug("Error detecting scripts in search request", e); |
| } | ||
| try { | ||
| if (detectScript(searchRequest)) { | ||
| scriptSearchCounter.add(1, Tags.create()); |
2 participants
Description
This will emit metric script_search_count when script query is made.
Sample metric
metric: ImmutableMetricData{resource=Resource{schemaUrl=null, attributes={service.name="OpenSearch"}}, instrumentationScopeInfo=InstrumentationScopeInfo{name=org.opensearch.telemetry, version=null, schemaUrl=null, attributes={}}, name=script_search_count, description=Count of search requests tagged by whether they contain a script, unit=count, type=DOUBLE_SUM, data=ImmutableSumData{points=[ImmutableDoublePointData{startEpochNanos=1773264920824685781, epochNanos=1773264980824673234, attributes={}, value=1.0, exemplars=[]}], monotonic=true, aggregationTemporality=DELTA}}