feat(ibm-mq-metrics): report queue manager uptime (#2823)#2835
feat(ibm-mq-metrics): report queue manager uptime (#2823)#2835harshitt13 wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new ibm.mq.queue_manager.uptime gauge that reports cumulative queue-manager uptime in seconds (derived from PCF MQCA_Q_MGR_START_DATE/MQCA_Q_MGR_START_TIME), and deprecates the existing ibm.mq.heartbeat metric in favor of it.
Changes:
- Adds
Metrics.createIbmMqQueueManagerUptime,MetricsConfig.isIbmMqQueueManagerUptimeEnabled, a newMessageBuddy.queueManagerUptimehelper, and emits the value inQueueManagerMetricsCollector. - Marks
Metrics.createIbmMqHeartbeatas@Deprecatedand wraps its consumer inWmqMonitorwith@SuppressWarnings("OtelDeprecatedApiUsage"). - Updates
model/metrics.yamlwith the new metric definition and a deprecation note onibm.mq.heartbeat.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ibm-mq-metrics/model/metrics.yaml | Declares the new uptime metric and notes deprecation of heartbeat. |
| ibm-mq-metrics/src/main/java/io/opentelemetry/ibm/mq/metrics/Metrics.java | Adds the uptime gauge factory and @Deprecated annotation on heartbeat. |
| ibm-mq-metrics/src/main/java/io/opentelemetry/ibm/mq/metrics/MetricsConfig.java | Adds an enable flag for the new metric. |
| ibm-mq-metrics/src/main/java/io/opentelemetry/ibm/mq/metricscollector/MessageBuddy.java | Parses PCF start date/time and computes uptime in seconds. |
| ibm-mq-metrics/src/main/java/io/opentelemetry/ibm/mq/metricscollector/QueueManagerMetricsCollector.java | Registers the gauge and emits the new metric inside the collection loop. |
| ibm-mq-metrics/src/main/java/io/opentelemetry/ibm/mq/WmqMonitor.java | Suppresses the deprecation warning for the still-active heartbeat gauge. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 895d64f00f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Please rebase, we have changed the way we record metrics to use a MetricProducer. |
Signed-off-by: Harshit Kushwaha <find.harshitkushwaha@gmail.com>
Signed-off-by: Harshit Kushwaha <find.harshitkushwaha@gmail.com>
…ime counter and update related metrics Signed-off-by: Harshit Kushwaha <find.harshitkushwaha@gmail.com>
harshitt13
force-pushed
the
feat/ibm-mq-uptime-2823
branch
from
6f8dc9f to
b9af0d0
Compare
…Producer for clarity Signed-off-by: Harshit Kushwaha <find.harshitkushwaha@gmail.com>
Rebased and refactored the implementation to utilize the new MetricProducer pattern as requested. |
| (k, v) -> { | ||
| if (v == null) { | ||
| return value; | ||
| } else { | ||
| return v + value; | ||
| } |
There was a problem hiding this comment.
nitpick:
| (k, v) -> { | |
| if (v == null) { | |
| return value; | |
| } else { | |
| return v + value; | |
| } | |
| (k, v) -> v == null ? value : v + value |
There was a problem hiding this comment.
Oh, is the value coming in a cumulative value already? If so, doesn't this math make it incorrect?
I'm thinking we should just take the value that we're given and report it as a gauge. I'm not sure that he guidance to use a counter is correct, merely because it's monotonic. Since we're reporting a "current value" of the measured uptime, wouldn't a gauge be better?
Also, what if the manager restarted between collects, causing its uptime to drop?
There was a problem hiding this comment.
You are right. Because the value is already an absolute cumulative value. Adding it to the previous value (v + value) across collection intervals was a logic error.
I agree that an asynchronous Gauge is correct here. By simply reporting the latest calculated value as a Gauge, it also perfectly handles the restart scenario you mentioned: if the queue manager restarts between collections, the new start_time will simply result in a smaller reported value, which a Gauge handles natively without issue.
I will push a fix to remove the addition logic so it just reports the latest measured value.
Signed-off-by: Harshit Kushwaha <find.harshitkushwaha@gmail.com>
Signed-off-by: Harshit Kushwaha <find.harshitkushwaha@gmail.com>
Description:
Feature addition - This PR introduces a new metric to report the cumulative uptime of the queue manager in seconds, providing better visibility into queue manager restarts compared to the static heartbeat metric.
ibm_mq.queue_manager.uptimemetric (Gauge, seconds) calculating uptime from PCF attributesCMQCFC.MQCACF_Q_MGR_START_DATEandCMQCFC.MQCACF_Q_MGR_START_TIME.ZoneOffset.UTC.heartbeatmetric per the issue discussion, keeping it active with@SuppressWarningsduring the transition period so users aren't immediately broken.Existing Issue(s):
Fixes #2823
Testing:
QueueManagerMetricsCollectorTestto inject mock start dates/times into the PCF response and verify the new gauge emission.MessageBuddyTestto explicitly verify that the uptime calculation remains perfectly stable regardless of the collector host's underlying JVM default timezone../gradlew :ibm-mq-metrics:generateWeaverand verified existing module tests pass cleanly.Documentation:
model/metrics.yamlandconfig.ymlto define and enable the newibm_mq.queue_manager.uptimemetric.@Deprecatedannotations and Javadoc for theheartbeatmetric.Outstanding items:
heartbeatmetric is currently deprecated but still emitting. It will need to be completely removed in a future major release.