mlbiscoc commented on PR #4284:
URL: https://github.com/apache/solr/pull/4284#issuecomment-4336969576
Hey @janhoy sorry, just realized I missed this PR.
> Do you agree with the name mappings above?
Mappings LGTM
> The two existing solr_zk_child_fetches and
solr_zk_cumulative_children_fetched both talk about the same thing, only
cumulative is total? So a bit strange that they use child_fetches vs
children_fetched. Should solr.zk.child.fetches instead be solr.zk.child_fetches
to align with solr_zk_cumulative_children_fetched? Or should we do a breaking
change here and align naming?
I took a deeper look at the collections and I don't think they talk about
the same thing and I named them incorrectly. `solr_zk_child_fetches` fetched
the number of `getChildren` calls:
`childFetches.increment();`
while `solr_zk_cumulative_children_fetched` got the number of actual child
nodes across all of the operations.
`cumulativeChildrenFetched.add(trace.getResponseChildrenCount());`
WDYT of `solr_zk_child_fetches` -> `solr.zk.get_children.ops` and
`solr_zk_cumulative_children_fetched` -> `solr.zk.children.fetched`? But then
also would someone even care about the total number of
`solr.zk.children_fetched` from a Solr perspective? I would probably just drop
that metric now that I think of it but idk maybe someone thinks otherwise.
> The crossdc consumer metrics do not have a solr. prefix. The corresponsing
producer metrics use solr.core.crossdc.producer.XXX. So ideally consumer would
also use solr.core.crossdc.consumer.XXX. Or why the core part in there? Should
we do a breaking change here, for one or for both?
CC @sigram. We should add `solr.` prefix. Tbh I don't know enough about the
crossdc module but to me seems like we don't need `core` there.
To answer your "breaking changes" question, yes I think we should just make
the changes. The mailing list was already sent out about breaking changes on
metrics so we should continue to go forward with it. I can see it being
debatable if we did a drastic metric name overhaul across everything but a few
here that are ZK metrics and the cross DC module should be fine.
--
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]
