[PR] fix(alert): mark proxy discovery failure as a whole-scope failure [rocketmq-dashboard]

["zjncs (via GitHub)" <gi...@apache.org>]

zjncs opened a new pull request, #4133:
URL: https://github.com/apache/rocketmq-dashboard/pull/4133

   ## Motivation
   
   `ApacheRocketMqProxyMetricsCollector.collect` returns a single unavailable sample with the placeholder label `proxyAddr=unknown` when proxy discovery fails for the whole instance (e.g. nameserver outage):
   
   ```java
   } catch (RuntimeException error) {
       return List.of(unavailable(instance, null, Map.of("proxyAddr", "unknown"), collectedAt));
   }
   ```
   
   #2957 added `NativeAlertProcessor.containsWholeScopeFailure` to skip reconciliation when a collection reports a whole-scope failure — detected as an unavailable sample with **empty labels** (the marker `ApacheRocketMqClusterMetricsCollector` already emits for nameserver failures). The proxy discovery-failure sample predates that convention (#2533) and was not aligned, so:
   
   1. `containsWholeScopeFailure` returns false (labels are non-empty) and `reconcileMissingActiveStates` runs;
   2. the placeholder fingerprint (`proxyAddr=unknown`) never matches the active states of the **real** proxies;
   3. every firing `proxy.availability` alert for the instance is **falsely resolved** — resolution events are emitted and notifications enqueued — while in reality the collector could not discover a single proxy and knows nothing about their status.
   
   So a nameserver hiccup turns all "proxy down" alerts into "resolved" notifications, masking real outages.
   
   ## Modification
   
   Emit the whole-scope failure marker (empty labels) for the discovery failure, matching `ApacheRocketMqClusterMetricsCollector`\'s nameserver failure sample and the detection added in #2957:
   
   ```java
   return List.of(unavailable(instance, null, Map.of(), collectedAt));
   ```
   
   The per-proxy failure paths (probe unreachable / probe error / malformed address) keep their real `proxyAddr` labels, so per-target unavailability still reconciles correctly and availability rules keep firing per real proxy. After discovery recovers, any state fired for the whole-scope marker carries the empty fingerprint and is resolved by the next successful reconciliation.
   
   ## Verification
   
   `mvn -f server/pom.xml test -Dtest='ApacheRocketMqProxyMetricsCollectorTest,NativeAlertProcessorTest'`
   
   fail-before (fix reverted, tests kept):
   
   ```
   ApacheRocketMqProxyMetricsCollectorTest.recordsWholeScopeFailureSampleWhenProxyDiscoveryFailsTest  <<< FAILURE!
   Expecting empty but was: {"proxyAddr"="unknown"}
   Tests run: 22, Failures: 1, Errors: 0
   ```
   
   pass-after:
   
   ```
   Tests run: 3, Failures: 0 ... in ApacheRocketMqProxyMetricsCollectorTest
   Tests run: 19, Failures: 0 ... in NativeAlertProcessorTest
   Tests run: 22, Failures: 0, Errors: 0
   BUILD SUCCESS
   ```
   
   Also updated the existing discovery-failure test to pin the whole-scope marker (it previously asserted the placeholder label) and added a processor-level test showing the firing state of a real proxy survives a discovery outage.
   
   Follow-up to #2957; no associated issue — found by code inspection.


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(alert): mark proxy discovery failure as a whole-scope failure [rocketmq-dashboard]

["lizhimins (via GitHub)" <gi...@apache.org>]

lizhimins closed pull request #4133: fix(alert): mark proxy discovery failure as a whole-scope failure
URL: https://github.com/apache/rocketmq-dashboard/pull/4133


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(alert): mark proxy discovery failure as a whole-scope failure [rocketmq-dashboard]

["lizhimins (via GitHub)" <gi...@apache.org>]

lizhimins commented on PR #4133:
URL: https://github.com/apache/rocketmq-dashboard/pull/4133#issuecomment-5602182252

   Closed as folded into #4119, merged into `rocketmq-studio` as f04dea2b.
   
   The production fix shipped exactly as you wrote it: the discovery-failure sample now carries empty labels instead of `proxyAddr=unknown`, so it lands in the whole-scope failure marker that #2957 established and `NativeAlertProcessor.containsWholeScopeFailure` skips reconciliation instead of resolving every firing proxy alert during an outage. Keeping the real `proxyAddr` on the three per-proxy failure paths was the right scoping — a single unreachable proxy should still reconcile normally. Your reasoning also matches `ApacheRocketMqClusterMetricsCollector`'s nameserver failure sample, so the two collectors are now consistent.
   
   One change was made during integration, to the tests rather than the fix. Your `ApacheRocketMqProxyMetricsCollectorTest` case is what pins this properly: it drives the real collector and asserts the emitted labels are empty, so reverting the fix turns it red. The companion case added to `NativeAlertProcessorTest` was dropped, because it constructed the empty-label `MetricSample` inline and therefore still passed with the collector fix reverted — it exercised the processor's pre-existing whole-scope handling, which `doesNotResolveMissingActiveStateWhenCollectionReportsWholeScopeUnavailableTest` already covers. Deleting it cost no coverage; keeping it would have implied the end-to-end path was pinned when it was not. `NativeAlertProcessorTest` runs 18 green and the collector test 3 green.
   
   For context on the grouping: these four all sit on the native alert evaluation path and each one alone leaves the others still misfiring, so they were integrated as one change on the target PR's branch, each kept as its own commit with you as the author.


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org