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

   ### Which Issue(s) This PR Fixes
   
   None filed — found while re-auditing the client diagnostics after #6633. 
Happy to file one if the convention requires it.
   
   ### What This PR Does
   
   `analyzeClientConnections` currently treats every proxy-derived row as two 
anomalies. The server deliberately leaves `protocol` and `version` null on rows 
collected through a proxy (`RocketMQClientProvider#toConnectionVO`: *"The Proxy 
response has no protocol"*), and the clients page already renders those rows 
with a dedicated unknown-protocol filter sentinel (`UNKNOWN_PROTOCOL` in 
`clients.tsx`). But the diagnostics normalized the null into the string 
`'unknown'` **before** the checks ran, so each proxy row fired:
   
   - `UNKNOWN_PROTOCOL` — warning, 9 score points, and
   - `UNKNOWN_VERSION` — warning, 9 score points.
   
   Three gRPC consumers behind a proxy therefore cost 54 score points and 
flipped the whole panel to `critical` / 「客户端连接存在高风险」 on a deployment where 
nothing is wrong, and the diagnostics table grew two warning rows per proxy 
connection (each opening a detail modal since #6633).
   
   The fix, in two parts:
   
   1. `analyzeClientConnections` now normalizes only the identity fields up 
front (`clientId` / `type` / `groupOrTopic` / `address`); `protocol` / 
`language` / `version` stay raw so the checks can tell "the Proxy response 
carries no value" apart from "a present-but-unrecognized value". Every other 
consumer of those fields already normalizes at point of use (`uniqueSorted` / 
`countBy` / the local `normalizeText` in `addUnknownFieldIssues`), so grouping, 
summaries and the language check are unchanged.
   2. `addUnknownFieldIssues` emits `UNKNOWN_PROTOCOL` / `UNKNOWN_VERSION` only 
when the field is actually present. An absent protocol/version is the expected 
proxy shape, not an anomaly.
   
   A present-but-unrecognized value still warns — the existing `protocol: 
'Custom'` / `version: '-'` case is unchanged and still covered by the 
pre-existing test.
   
   ### Verification
   
   - Red (unpatched trunk): the new test fails with `expected [ 
'UNKNOWN_PROTOCOL', …(5) ] to not include 'UNKNOWN_PROTOCOL'` — 6 warnings, 
score 46, overall status `critical`.
   - Green (this patch): 0 issues, score 100, status `healthy`.
   - `npx vitest run src/utils/clientConnectionDiagnostics.test.ts` — **8/8** 
(7 pre-existing + 1 new).
   - `npx vitest run src/pages/cluster/__tests__/ClientsPage.test.tsx` — 
**25/25**, including the "renders missing Proxy metadata" and "filters 
protocol-less proxy rows" tests.
   - `npx eslint` clean on both touched files; `tsc -b` clean; Prettier clean 
on the staged blobs.
   


-- 
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]

Reply via email to