SOLR-18165 Complete dot-separated metric name migration#4284
Conversation
There was a problem hiding this comment.
Pull request overview
Completes the remaining OpenTelemetry metric instrument renames from underscore-delimited names to dot-separated names as part of SOLR-18165.
Changes:
- Migrates
AuditLoggerPluginmetric names tosolr.auditlogger.*. - Migrates
CaffeineCachemetric base name and suffixes tosolr.caffeine_cache.*. - Migrates ZooKeeper metrics in
ZkContainertosolr.zk.*, and cross-dc consumer metrics tocrossdc.consumer.*. - Adds an unreleased changelog entry for this follow-up migration.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| solr/cross-dc-manager/src/java/org/apache/solr/crossdc/manager/consumer/OtelMetrics.java | Renames cross-dc consumer OTel instrument names to dot-separated format. |
| solr/core/src/java/org/apache/solr/security/AuditLoggerPlugin.java | Renames audit logger OTel instrument names to solr.auditlogger.*. |
| solr/core/src/java/org/apache/solr/search/CaffeineCache.java | Renames caffeine cache OTel instrument base name and suffixes to dot-separated format. |
| solr/core/src/java/org/apache/solr/core/ZkContainer.java | Renames ZooKeeper OTel instrument names to solr.zk.*. |
| changelog/unreleased/SOLR-18165-More-dot-separated-metric-names.yml | Adds changelog entry documenting the additional metric renames. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@chan-dx I ping you here since I could not add you as reviewer... |
|
Hey @janhoy sorry, just realized I missed this PR.
Mappings LGTM
I took a deeper look at the collections and I don't think they talk about the same thing and I named them incorrectly. while WDYT of
CC @sigram. We should add 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. |
Remove solr_zk_cumulative_children_fetched
Proposal: Rename CrossDC metric namesFollowing @mlbiscoc's suggestion — add Consumer:
|
| Old | New |
|---|---|
crossdc.consumer.input.msg.total |
solr.crossdc.consumer.input.msg.total |
crossdc.consumer.input.req.total |
solr.crossdc.consumer.input.req.total |
crossdc.consumer.collapsed.total |
solr.crossdc.consumer.collapsed.total |
crossdc.consumer.output.total |
solr.crossdc.consumer.output.total |
crossdc.consumer.output.batch_size |
solr.crossdc.consumer.output.batch_size |
crossdc.consumer.output.backoff_time |
solr.crossdc.consumer.output.backoff_time |
crossdc.consumer.output.time |
solr.crossdc.consumer.output.time |
crossdc.consumer.output.first_attempt_time |
solr.crossdc.consumer.output.first_attempt_time |
Prometheus gains a solr_ prefix: crossdc_consumer_output_total → solr_crossdc_consumer_output_total
Producer: solr.core.crossdc.producer. → solr.crossdc.producer.
| Old | New |
|---|---|
solr.core.crossdc.producer.local.processed |
solr.crossdc.producer.local.processed |
solr.core.crossdc.producer.submitted |
solr.crossdc.producer.submitted |
solr.core.crossdc.producer.submitted.add |
solr.crossdc.producer.submitted.add |
solr.core.crossdc.producer.submitted.delete_by_id |
solr.crossdc.producer.submitted.delete_by_id |
solr.core.crossdc.producer.submitted.delete_by_query |
solr.crossdc.producer.submitted.delete_by_query |
solr.core.crossdc.producer.submitted.commit |
solr.crossdc.producer.submitted.commit |
solr.core.crossdc.producer.document_size |
solr.crossdc.producer.document_size |
solr.core.crossdc.producer.doc_too_large_errors |
solr.crossdc.producer.doc_too_large_errors |
Prometheus: solr_core_crossdc_producer_* → solr_crossdc_producer_*
|
@mlbiscoc yes, it makes sense to drop the "core" segment in the names. |
…re. from producer Consumer: crossdc.consumer.* → solr.crossdc.consumer.* Producer: solr.core.crossdc.producer.* → solr.crossdc.producer.* Both sides are now symmetric under solr.crossdc.*
|
I pushed the crossdc changes. Will merge and backport this PR to 10x next week Tuesday if no objections. |
mlbiscoc
left a comment
There was a problem hiding this comment.
LGTM! Thanks Jan for cleaning up some of my mess :)
Not at all Matthew :) Looking forward to completing the new grafana dashboards... |
(cherry picked from commit 193b2e5)
https://issues.apache.org/jira/browse/SOLR-18165
PR #4223 converted the majority of Solr metric instrument names from
underscore_delimitedto
dot.separatedOTel standard naming. Several files were missed or only partially converted.This PR completes that work.
Files changed
AuditLoggerPlugin.java— all 7 metric names were still in oldsolr_auditlogger_*format;converted to
solr.auditlogger.*.CaffeineCache.java— the default base metric name"solr_caffeine_cache"and the fivemetric name suffixes (
_lookups,_ops,_size,_ram_used,_warmup_time) were still usingunderscores as namespace separators. Converted to
"solr.caffeine_cache"base withdot-prefixed suffixes (
.lookups,.ops,.size,.ram_used,.warmup_time).ZkContainer.java— all 7 metrics still in oldsolr_zk_*format; converted tosolr.zk.*.OtelMetrics.java(cross-dc-manager) — all 8 metric names were in oldcrossdc_consumer_*format. The
NAME_PREFIXconstant is updated to"crossdc.consumer."and all suffix stringsare converted to dot-separated.
Metric name mapping
solr_auditlogger_countsolr.auditlogger.countsolr_auditlogger_errorssolr.auditlogger.errorssolr_auditlogger_lostsolr.auditlogger.lostsolr_auditlogger_request_timessolr.auditlogger.request_timessolr_auditlogger_queuesolr.auditlogger.queuesolr_auditlogger_queued_timesolr.auditlogger.queued_timesolr_auditlogger_async_enabledsolr.auditlogger.async_enabledsolr_caffeine_cache_lookupssolr.caffeine_cache.lookupscaffeine_cacheis the cache type namesolr_caffeine_cache_opssolr.caffeine_cache.opssolr_caffeine_cache_sizesolr.caffeine_cache.sizesolr_caffeine_cache_ram_usedsolr.caffeine_cache.ram_usedsolr_caffeine_cache_warmup_timesolr.caffeine_cache.warmup_timesolr_zk_opssolr.zk.opssolr_zk_readsolr.zk.readsolr_zk_watches_firedsolr.zk.watches_firedsolr_zk_writtensolr.zk.writtensolr_zk_cumulative_multi_ops_totalsolr.zk.cumulative.multi_ops.totalsolr_zk_child_fetchessolr.zk.child.fetchessolr_zk_cumulative_children_fetchedsolr.zk.cumulative.children_fetchedcrossdc_consumer_input_msg_totalcrossdc.consumer.input.msg.total_total?crossdc_consumer_input_req_totalcrossdc.consumer.input.req.totalcrossdc_consumer_collapsed_totalcrossdc.consumer.collapsed.totalcrossdc_consumer_output_totalcrossdc.consumer.output.totalcrossdc_consumer_output_batch_sizecrossdc.consumer.output.batch_sizecrossdc_consumer_output_backoff_timecrossdc.consumer.output.backoff_timecrossdc_consumer_output_timecrossdc.consumer.output.timecrossdc_consumer_output_first_attempt_timecrossdc.consumer.output.first_attempt_timesolr_zk_child_fetchessolr.zk.get_children.opssolr_zk_cumulative_children_fetchedQuestions:
solr_zk_child_fetchesandsolr_zk_cumulative_children_fetchedboth talk about the same thing, only cumulative is total? So a bit strange that they usechild_fetchesvschildren_fetched. Shouldsolr.zk.child.fetchesinstead besolr.zk.child_fetchesto align withsolr_zk_cumulative_children_fetched? Or should we do a breaking change here and align naming?solr.prefix. The corresponsing producer metrics usesolr.core.crossdc.producer.XXX. So ideally consumer would also usesolr.core.crossdc.consumer.XXX. Or why the core part in there? Should we do a breaking change here, for one or for both?