Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy merged PR #4223: URL: https://github.com/apache/solr/pull/4223 -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on PR #4223: URL: https://github.com/apache/solr/pull/4223#issuecomment-4198770005 I synced this with main and intend to merge tomorrow, unless there are additional comments. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r3044801897 ## solr/core/src/java/org/apache/solr/update/UpdateLog.java: ## @@ -665,20 +665,20 @@ public void initializeMetrics(SolrMetricsContext parentContext, Attributes attri applyingBufferedOpsCounter = new AttributedLongCounter( solrMetricsContext.longCounter( -"solr_core_update_log_applied_buffered_ops", +"solr.core.update_log.applied_buffered_ops", "Total number of buffered operations applied"), baseAttributes); replayOpsCounter = new AttributedLongCounter( solrMetricsContext.longCounter( -"solr_core_update_log_replay_ops", "Total number of log replay operations"), +"solr.core.update_log.replay.ops", "Total number of log replay operations"), Review Comment: Sorry, this has been discussed before and was explicitly changed to current state. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r3044745960 ## solr/core/src/java/org/apache/solr/update/UpdateLog.java: ## @@ -665,20 +665,20 @@ public void initializeMetrics(SolrMetricsContext parentContext, Attributes attri applyingBufferedOpsCounter = new AttributedLongCounter( solrMetricsContext.longCounter( -"solr_core_update_log_applied_buffered_ops", +"solr.core.update_log.applied_buffered_ops", "Total number of buffered operations applied"), baseAttributes); replayOpsCounter = new AttributedLongCounter( solrMetricsContext.longCounter( -"solr_core_update_log_replay_ops", "Total number of log replay operations"), +"solr.core.update_log.replay.ops", "Total number of log replay operations"), Review Comment: Should this be `solr.core.update_log.replay_ops` ? -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2987353113 ## solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc: ## @@ -292,6 +292,28 @@ Attention: == Solr 10.1 +=== OpenTelemetry + +The new OTEL based metric system introduced in 10.0 is considered BETA, and some breaking changes are to be expected in this and coming minor releases. Review Comment: I restructured major changes under a common "OpenTelemetry" heading, and re-iterate that it is BETA, in case someone using 10.0 wants to upgrade and did not catch the retroactive BETA decision. So when they read the 10.1 notes they will be notified. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2987293257 ## solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc: ## @@ -284,3 +284,26 @@ Attention: * The `llm` module has been renamed to `language-models`. * The HNSW parameters `hnswMaxConnections` and `hnswBeamWidth` have been renamed to `hnswM` and `hnswEfConstruction`, respectively, so they must be updated accordingly in the schema.xml file. + +== Solr 10.1 + +=== OTEL metrics classified as BETA + +The new OTEL metrics system, naming and dashboard should be treated as a BETA. We reserve the right to make breaking changes in this space in minor versions. Review Comment: Removed again due to #4235 -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2987217424 ## solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc: ## @@ -284,3 +284,26 @@ Attention: * The `llm` module has been renamed to `language-models`. * The HNSW parameters `hnswMaxConnections` and `hnswBeamWidth` have been renamed to `hnswM` and `hnswEfConstruction`, respectively, so they must be updated accordingly in the schema.xml file. + +== Solr 10.1 + +=== OTEL metrics classified as BETA + +The new OTEL metrics system, naming and dashboard should be treated as a BETA. We reserve the right to make breaking changes in this space in minor versions. + +=== OTLP Metric Names Changed to Dot-Separated Format + +*Breaking change for OTLP consumers.* + +Solr's metric names exported via OTLP now follow the https://opentelemetry.io/docs/specs/semconv/general/naming/[OpenTelemetry semantic convention] of using dot-separated names instead of the underscore-separated names used in Solr 10.0. + +For example, `solr_core_requests` is now `solr.core.requests`, and `solr_node_executor` is now `solr.node.executor`. + +*Prometheus output is unaffected.* The OTel Prometheus exporter automatically converts dots to underscores, so the `/admin/metrics?wt=prometheus` endpoint continues to produce the same metric names as before (e.g., `solr_core_requests_total`). + +*Workaround for OTLP consumers depending on the old names:* +Users who consume Solr metrics via OTLP and rely on the 10.0 underscore-format names can use metric renaming or transformation features in their OpenTelemetry Collector pipeline to convert the new dot-separated names back to the old format during the transition. + +=== Metric solr_core_indexsearcher_open_warmup_time is deprecated + +If you use `solr_core_indexsearcher_open_warmup_time` in a dashboard, it duplicates `solr_core_indexsearcher_warmup_time` and is deprecated for removal in a later 10.x release. Review Comment: Since we'll declare the entire metrics system as BETA I'll not go the deprecation route here but just remove it. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2970510943 ## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ## @@ -2637,14 +2637,14 @@ public void initializeMetrics(SolrMetricsContext solrMetricsContext, Attributes warmupTimer = new AttributedLongTimer( solrMetricsContext.longHistogram( -"solr_core_indexsearcher_warmup_time", +"solr.core.indexsearcher.warmup.time", Review Comment: I started a dev list thread about this aiming to gather support via (lazy) consensus and then modify docs. In that case, we can delete the duplicate metric below instead of deprecating it. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
dsmiley commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2968646351 ## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ## @@ -2637,14 +2637,14 @@ public void initializeMetrics(SolrMetricsContext solrMetricsContext, Attributes warmupTimer = new AttributedLongTimer( solrMetricsContext.longHistogram( -"solr_core_indexsearcher_warmup_time", +"solr.core.indexsearcher.warmup.time", Review Comment: IMO there's nothing to announce. 10 is fresh... early adopters. Relatively small technically backwards incompatible things are gonna change. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
mlbiscoc commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2968645017 ## solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc: ## @@ -284,3 +284,26 @@ Attention: * The `llm` module has been renamed to `language-models`. * The HNSW parameters `hnswMaxConnections` and `hnswBeamWidth` have been renamed to `hnswM` and `hnswEfConstruction`, respectively, so they must be updated accordingly in the schema.xml file. + +== Solr 10.1 + +=== OTEL metrics classified as BETA + +The new OTEL metrics system, naming and dashboard should be treated as a BETA. We reserve the right to make breaking changes in this space in minor versions. + +=== OTLP Metric Names Changed to Dot-Separated Format + +*Breaking change for OTLP consumers.* + +Solr's metric names exported via OTLP now follow the https://opentelemetry.io/docs/specs/semconv/general/naming/[OpenTelemetry semantic convention] of using dot-separated names instead of the underscore-separated names used in Solr 10.0. + +For example, `solr_core_requests` is now `solr.core.requests`, and `solr_node_executor` is now `solr.node.executor`. + +*Prometheus output is unaffected.* The OTel Prometheus exporter automatically converts dots to underscores, so the `/admin/metrics?wt=prometheus` endpoint continues to produce the same metric names as before (e.g., `solr_core_requests_total`). + +*Workaround for OTLP consumers depending on the old names:* +Users who consume Solr metrics via OTLP and rely on the 10.0 underscore-format names can use metric renaming or transformation features in their OpenTelemetry Collector pipeline to convert the new dot-separated names back to the old format during the transition. + +=== Metric solr_core_indexsearcher_open_warmup_time is deprecated + +If you use `solr_core_indexsearcher_open_warmup_time` in a dashboard, it duplicates `solr_core_indexsearcher_warmup_time` and is deprecated for removal in a later 10.x release. Review Comment: In this case, we should also then update the `# HELP` description in the metric to say deprecated. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
mlbiscoc commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2968642856 ## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ## @@ -2637,14 +2637,14 @@ public void initializeMetrics(SolrMetricsContext solrMetricsContext, Attributes warmupTimer = new AttributedLongTimer( solrMetricsContext.longHistogram( -"solr_core_indexsearcher_warmup_time", +"solr.core.indexsearcher.warmup.time", Review Comment: Yes I like this idea. I can imagine us continuing to need more changes as OTEL is adopted over time and until we are in a stable state, it would be difficult to continue. I can start an email thread on the DEV list for anyones thoughts. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2968557948 ## solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc: ## @@ -284,3 +284,26 @@ Attention: * The `llm` module has been renamed to `language-models`. * The HNSW parameters `hnswMaxConnections` and `hnswBeamWidth` have been renamed to `hnswM` and `hnswEfConstruction`, respectively, so they must be updated accordingly in the schema.xml file. + +== Solr 10.1 + +=== OTEL metrics classified as BETA + +The new OTEL metrics system, naming and dashboard should be treated as a BETA. We reserve the right to make breaking changes in this space in minor versions. + +=== OTLP Metric Names Changed to Dot-Separated Format + +*Breaking change for OTLP consumers.* + +Solr's metric names exported via OTLP now follow the https://opentelemetry.io/docs/specs/semconv/general/naming/[OpenTelemetry semantic convention] of using dot-separated names instead of the underscore-separated names used in Solr 10.0. + +For example, `solr_core_requests` is now `solr.core.requests`, and `solr_node_executor` is now `solr.node.executor`. + +*Prometheus output is unaffected.* The OTel Prometheus exporter automatically converts dots to underscores, so the `/admin/metrics?wt=prometheus` endpoint continues to produce the same metric names as before (e.g., `solr_core_requests_total`). + +*Workaround for OTLP consumers depending on the old names:* +Users who consume Solr metrics via OTLP and rely on the 10.0 underscore-format names can use metric renaming or transformation features in their OpenTelemetry Collector pipeline to convert the new dot-separated names back to the old format during the transition. + +=== Metric solr_core_indexsearcher_open_warmup_time is deprecated + +If you use `solr_core_indexsearcher_open_warmup_time` in a dashboard, it duplicates `solr_core_indexsearcher_warmup_time` and is deprecated for removal in a later 10.x release. Review Comment: I did not remove the duplicate but deprecate it. With the BETA regime in place we can kill this in a minor version. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2968556515 ## solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc: ## @@ -284,3 +284,26 @@ Attention: * The `llm` module has been renamed to `language-models`. * The HNSW parameters `hnswMaxConnections` and `hnswBeamWidth` have been renamed to `hnswM` and `hnswEfConstruction`, respectively, so they must be updated accordingly in the schema.xml file. + +== Solr 10.1 + +=== OTEL metrics classified as BETA + +The new OTEL metrics system, naming and dashboard should be treated as a BETA. We reserve the right to make breaking changes in this space in minor versions. Review Comment: I added a BETA declaration her as proposed. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2967892169 ## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ## @@ -2637,14 +2637,14 @@ public void initializeMetrics(SolrMetricsContext solrMetricsContext, Attributes warmupTimer = new AttributedLongTimer( solrMetricsContext.longHistogram( -"solr_core_indexsearcher_warmup_time", +"solr.core.indexsearcher.warmup.time", Review Comment: Thanks. I'll fix this. IMO we should have classified the new OTEL metrics system as experimental/beta in 10.0 to allow for stabilization like this. Perhaps we can degrade it to BETA quality now by editing the 10.0 docs and sending a mail to the lists? Then we'll have a few releases to stabilize metrics, dashbaords etc. If you fancy that suggestion, would you want to run with it, separate from this PR? -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
mlbiscoc commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2967534936 ## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ## @@ -2637,14 +2637,14 @@ public void initializeMetrics(SolrMetricsContext solrMetricsContext, Attributes warmupTimer = new AttributedLongTimer( solrMetricsContext.longHistogram( -"solr_core_indexsearcher_warmup_time", +"solr.core.indexsearcher.warmup.time", Review Comment: @chan-dx Thanks for finding this and also +1 in your suggested change! @janhoy I did some digging from when I did the OTEL migration a while ago. I think my migration here was redundant. Basically when we had Dropwizard implementation, both these metrics did exist, but one was a `timer` and the other was a `gauge` which I converted to a timer. The one in `SolrIndexSearcher` was the gauge and just gave the last seen time. So they are essentially measuring the same thing now and is just redundant. I'm thinking we remove the one in SolrCore and have SolrIndexSearcher just be the only metric existing giving warmup time. This does break anyone then using that metric, but we should just point them to use the one in SolrIndexSearcher instead in the `changelog` unless this isn't acceptable for 10.x. WDYT? -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2965367586 ## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ## @@ -2637,14 +2637,14 @@ public void initializeMetrics(SolrMetricsContext solrMetricsContext, Attributes warmupTimer = new AttributedLongTimer( solrMetricsContext.longHistogram( -"solr_core_indexsearcher_warmup_time", +"solr.core.indexsearcher.warmup.time", Review Comment: Interesting. I vote +1 for your suggested change. But can you @mlbiscoc explain what the difference between `solr.core.indexsearcher.warmup.time` and `solr.core.indexsearcher.open.warmup_time` is in the first place? -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
chan-dx commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2965141952 ## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ## @@ -2637,14 +2637,14 @@ public void initializeMetrics(SolrMetricsContext solrMetricsContext, Attributes warmupTimer = new AttributedLongTimer( solrMetricsContext.longHistogram( -"solr_core_indexsearcher_warmup_time", +"solr.core.indexsearcher.warmup.time", Review Comment: I noticed a small inconsistency between two warmup-related metric names: - `SolrIndexSearcher.java` uses `solr.core.indexsearcher.warmup.time` - `SolrCore.java` uses `solr.core.indexsearcher.open.warmup_time` Should these be aligned? ```suggestion "solr.core.indexsearcher.warmup_time", ``` I couldn’t spot any other metrics under a `warmup.*` pattern in `SolrIndexSearcher.java`, so `warmup_time` felt more natural to me here, since “warmup time” reads like a single compound concept and that’s already the form used in `open.warmup_time`. But if `warmup.time` is intentional, please ignore this. I just wanted to highlight this in case it wasn’t intentional. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
chan-dx commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2965141952 ## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ## @@ -2637,14 +2637,14 @@ public void initializeMetrics(SolrMetricsContext solrMetricsContext, Attributes warmupTimer = new AttributedLongTimer( solrMetricsContext.longHistogram( -"solr_core_indexsearcher_warmup_time", +"solr.core.indexsearcher.warmup.time", Review Comment: I noticed a small inconsistency between two warmup-related metric names: - `SolrIndexSearcher.java` uses `solr.core.indexsearcher.warmup.time` - `SolrCore.java` uses `solr.core.indexsearcher.open.warmup_time` Should these be aligned? ```suggestion solrMetricsContext.longHistogram( "solr.core.indexsearcher.warmup_time" ``` I couldn’t spot any other metrics under a `warmup.*` pattern in `SolrIndexSearcher.java`, so `warmup_time` felt more natural to me here, since “warmup time” reads like a single compound concept and that’s already the form used in `open.warmup_time`. But if `warmup.time` is intentional, please ignore this. I just wanted to highlight this in case it wasn’t intentional. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
mlbiscoc commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2961588173 ## solr/core/src/java/org/apache/solr/update/UpdateLog.java: ## @@ -637,22 +637,22 @@ public void initializeMetrics(SolrMetricsContext parentContext, Attributes attri observables.add( solrMetricsContext.observableLongGauge( -"solr_core_update_log_buffered_ops", +"solr.core.update_log.buffered.ops", "The current number of buffered operations", (observableLongMeasurement -> observableLongMeasurement.record(computeBufferedOps(), baseAttributes; observables.add( solrMetricsContext.observableLongGauge( -"solr_core_update_log_replay_logs_remaining", +"solr.core.update_log.replay.logs_remaining", Review Comment: Actually I think this is right now that I look at this again. Keep this as is but my suggestion below I think this needs to change. `Replay` is kind of a category. ## solr/core/src/java/org/apache/solr/update/UpdateLog.java: ## @@ -672,20 +672,20 @@ public void initializeMetrics(SolrMetricsContext parentContext, Attributes attri applyingBufferedOpsCounter = new AttributedLongCounter( solrMetricsContext.longCounter( -"solr_core_update_log_applied_buffered_ops", +"solr.core.update_log.applied_buffered_ops", "Total number of buffered operations applied"), baseAttributes); replayOpsCounter = new AttributedLongCounter( solrMetricsContext.longCounter( -"solr_core_update_log_replay_ops", "Total number of log replay operations"), +"solr.core.update_log.replay_ops", "Total number of log replay operations"), Review Comment: ```suggestion "solr.core.update_log.replay.ops", "Total number of log replay operations"), ``` -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2960848441 ## solr/core/src/java/org/apache/solr/update/UpdateLog.java: ## @@ -637,22 +637,22 @@ public void initializeMetrics(SolrMetricsContext parentContext, Attributes attri observables.add( solrMetricsContext.observableLongGauge( -"solr_core_update_log_buffered_ops", +"solr.core.update_log.buffered.ops", "The current number of buffered operations", (observableLongMeasurement -> observableLongMeasurement.record(computeBufferedOps(), baseAttributes; observables.add( solrMetricsContext.observableLongGauge( -"solr_core_update_log_replay_logs_remaining", +"solr.core.update_log.replay.logs_remaining", Review Comment: Probably. Unless `replay` is a category label with several other children I think this is the correct. Probably other examples as well. PS: You can use the "suggestions" button when commenting to suggest edits inline the comment. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
janhoy commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2960848441 ## solr/core/src/java/org/apache/solr/update/UpdateLog.java: ## @@ -637,22 +637,22 @@ public void initializeMetrics(SolrMetricsContext parentContext, Attributes attri observables.add( solrMetricsContext.observableLongGauge( -"solr_core_update_log_buffered_ops", +"solr.core.update_log.buffered.ops", "The current number of buffered operations", (observableLongMeasurement -> observableLongMeasurement.record(computeBufferedOps(), baseAttributes; observables.add( solrMetricsContext.observableLongGauge( -"solr_core_update_log_replay_logs_remaining", +"solr.core.update_log.replay.logs_remaining", Review Comment: Probably. Unless `replay` is a category label with several other children I think this is the correct. Probably other examples as well. -- 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]
Re: [PR] SOLR-18165 Use dot separated metric names [solr]
mlbiscoc commented on code in PR #4223: URL: https://github.com/apache/solr/pull/4223#discussion_r2960517723 ## solr/core/src/java/org/apache/solr/update/UpdateLog.java: ## @@ -637,22 +637,22 @@ public void initializeMetrics(SolrMetricsContext parentContext, Attributes attri observables.add( solrMetricsContext.observableLongGauge( -"solr_core_update_log_buffered_ops", +"solr.core.update_log.buffered.ops", "The current number of buffered operations", (observableLongMeasurement -> observableLongMeasurement.record(computeBufferedOps(), baseAttributes; observables.add( solrMetricsContext.observableLongGauge( -"solr_core_update_log_replay_logs_remaining", +"solr.core.update_log.replay.logs_remaining", Review Comment: Should the separator between replay and logs be a `_`. -> `...replay_logs_remaining` -- 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]
