You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "dlmarion (via GitHub)" <gi...@apache.org> on 2026/05/11 14:36:13 UTC
[PR] Enable metrics by default to enable correct function of Monitor [accumulo]
dlmarion opened a new pull request, #6364:
URL: https://github.com/apache/accumulo/pull/6364
This change removes the LoggingMeterRegistryFactory from the default value of the general.micrometer.factory property and changes the default value of general.micrometer.enabled from false to true.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Enable metrics by default to enable correct function of Monitor [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #6364:
URL: https://github.com/apache/accumulo/pull/6364#discussion_r3243344582
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -369,7 +373,7 @@ temporary files (for example, when creating a pre-split table). \
'log4j2' or 'logback'.
""", "2.1.4"),
GENERAL_MICROMETER_FACTORY("general.micrometer.factory",
- "org.apache.accumulo.core.spi.metrics.LoggingMeterRegistryFactory",
+ "org.apache.accumulo.core.metrics.AccumuloMonitorMeterRegistryFactory",
Review Comment:
May want to put this in the spi package where the current Logging one exists, just because this package isn't public API.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Enable metrics by default to enable correct function of Monitor [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #6364:
URL: https://github.com/apache/accumulo/pull/6364
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Enable metrics by default to enable correct function of Monitor [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #6364:
URL: https://github.com/apache/accumulo/pull/6364#discussion_r3243867389
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -369,7 +373,7 @@ temporary files (for example, when creating a pre-split table). \
'log4j2' or 'logback'.
""", "2.1.4"),
GENERAL_MICROMETER_FACTORY("general.micrometer.factory",
- "org.apache.accumulo.core.spi.metrics.LoggingMeterRegistryFactory",
+ "org.apache.accumulo.core.metrics.AccumuloMonitorMeterRegistryFactory",
Review Comment:
Moved the AccumuloMonitorMeterRegistryFactory to the spi package in c7aa5df
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Enable metrics by default to enable correct function of Monitor [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #6364:
URL: https://github.com/apache/accumulo/pull/6364#issuecomment-4440984985
Commit 412d9d2 resolves some of the issues from the prior comment. Specifically, it does the following:
- Creates a MeterRegistryFactory for the MonitorMeterRegistry so it can be configured independently.
- Modifies MiniAccumuloConfigImpl to disable metrics.
- Modifies ITs that use metrics to fully set them up.
- Fixes an issue where cache and fate metrics were being emitted when metrics were disabled.
- Modifies AbstractServer.getMetrics to only emit metrics if the MonitorMeterRegistry is configured.
- Removed metrics configuration from some ITs that were not using them.
@ctubbsii and I had a discussion about the behavior of enabling / disabling metrics totally using the property `GENERAL_MICROMETER_ENABLED`. If this property is set to `false`, then no metrics are set up at all. There was a question as to whether this property should only apply to Accumulo related metrics. If so, then the properties `GENERAL_MICROMETER_JVM_METRICS_ENABLED` and `GENERAL_MICROMETER_LOG_METRICS` would act independently. I think it's fine to leave things as they are. If someone wants to set up the global registry using a Java Agent or Spring Boot, they can add the JVM and log metrics at that time and disable ours.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org