mlbiscoc commented on code in PR #3734:
URL: https://github.com/apache/solr/pull/3734#discussion_r2406518214
##########
solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java:
##########
Review Comment:
> No need to slightly vary descriptions; core vs node can be implicit based
on the metric name structure.
I wrote this a while ago and my view of this has kind of changed since then.
How about to reduce some of the duplicate code here, we merge these 2 metrics
names together:
```
solr_core_requests_total
solr_node_requests_total
```
into just
```
solr_requests_total
```
If there are core attributes then you know it was requests to a core. No
core level attributes, then it has to mean node level requests as there is
nothing to aggregate on it and we just need 1 metric name.
Example
```
solr_requests_total{handler="/select",core="foo"} 5
solr_requests_total{handler="/select",core="bar"} 5
solr_requests_total{handler="/select"} 10 <- Node level
```
> I feel like there should be a kind of inherit-aware factory of instruments
(that support it; no gauges) so that all inherit-possible metrics a core might
want to register (not just request handlers) could support it automatically. I
sympathize I may be suggesting massive scope creep...
Definitely scope creep but at the same time I see value in what you are
saying here. I'd have to see what implementing this would look like but off the
top of my head it'd be a somewhat big refactor. Could we push this back to a
10.x release? You can still calculate your node level requests by summing all
the cores in your backend or at aggregation. This is a feature that could very
well help your backend aggregation load but doesn't seem like a necessary thing
now. Also working on separate branch right now that is removing Dropwizard that
is taking time.
I can see about renaming requests to `solr_requests_total` so that in 10.x
its easy to just add this node level recording later if you agree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
