Copilot commented on code in PR #13141:
URL: https://github.com/apache/cloudstack/pull/13141#discussion_r3209616709
##########
server/src/main/java/com/cloud/server/StatsCollector.java:
##########
@@ -410,6 +415,11 @@ public boolean start() {
registerAll("memory", new MemoryUsageGaugeSet(), METRIC_REGISTRY);
registerAll("threads", new ThreadStatesGaugeSet(), METRIC_REGISTRY);
registerAll("jvm", new JvmAttributeGaugeSet(), METRIC_REGISTRY);
+ try {
+
JmxReporter.forRegistry(METRIC_REGISTRY).inDomain("vm-extra").build().start();
+ } catch (Exception e) {
+ logger.warn("Failed to start JMX reporter for METRIC_REGISTRY,
metrics will not be visible via JMX", e);
+ }
Review Comment:
The JMX reporter is started but the created JmxReporter instance isn’t
retained anywhere, so it can’t be stopped during `stop()` (and repeated
`start()` calls can attempt to re-register the same MBeans). Consider storing
the reporter in a field, guarding start so it’s only started once, and stopping
it (unregistering MBeans) in `stop()` to avoid resource/MBean leaks.
##########
server/src/main/java/com/cloud/server/StatsCollector.java:
##########
@@ -387,7 +388,11 @@ public String toString() {
private boolean _dailyOrHourly = false;
protected long managementServerNodeId =
ManagementServerNode.getManagementServerId();
protected long msId = managementServerNodeId;
- final static MetricRegistry METRIC_REGISTRY = new MetricRegistry();
+ public static final MetricRegistry METRIC_REGISTRY = new MetricRegistry();
+
+ public static void registerMetric(String name, Metric metric) {
+ METRIC_REGISTRY.register(name, metric);
+ }
Review Comment:
`registerMetric` accepts any `Metric`, but StatsCollector’s internal
harvesting logic only iterates over `METRIC_REGISTRY.getGauges()` (so non-Gauge
metrics registered here won’t be included in the collected
metricDetails/extracted fields). Either constrain this helper to registering
Gauges (or add a clearly documented contract that it’s intended for JMX/export
only), to avoid confusing API behavior for external callers.
--
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]
