unbridled-41 opened a new pull request, #4902:
URL: https://github.com/apache/rocketmq-dashboard/pull/4902
<!-- Base branch: `rocketmq-studio`, the RocketMQ Studio trunk. -->
### Which Issue(s) This PR Fixes
- No issue tracks this defect; the searches under *Duplicate check* found
none.
### Brief Description
For Tencent and Aliyun instances, every row of the 消费进度 (queue progress)
table reported
`Broker Offset 0` and `Consumer Offset 0` as if they were measurements,
although neither API can
report per-queue offsets at all.
```java
//
server/src/main/java/org/apache/rocketmq/studio/provider/tencent/TencentInstanceProvider.java:533-540
(base)
rows.add(QueueProgressVO.builder()
.topic(subscription.getTopic())
.broker("topic:" + subscription.getTopic())
.queueId(0)
.brokerOffset(0L) // fabricated: the API exposes only the
per-topic lag
.consumerOffset(0L)
.diffTotal(subscription.getConsumerLag() == null ? 0L :
subscription.getConsumerLag())
.build());
```
```java
//
server/src/main/java/org/apache/rocketmq/studio/provider/alibaba/AliyunConverters.java:191-198
and :206-212 (base)
rows.add(QueueProgressVO.builder()
.topic(entry.getKey())
.broker("topic:" + entry.getKey())
.queueId(0)
.brokerOffset(0L)
.consumerOffset(0L)
.diffTotal(ready)
.build());
```
`QueueProgressVO` has no way to say "unknown" for an offset, so zero was the
only value the
providers could send. The page renders those offsets with
`offset.toLocaleString()`
(`web/src/pages/instance/consumer.tsx:1295-1309`), so the row showed a real
lag next to two zeros
that contradict it, and the same value populated the reset-offset preview's
current offset.
The codebase already has the convention for this: a negative number means
"cannot be determined"
— `ConsumerLagResolver.UNKNOWN = -1` (whose Javadoc names the very problem,
"a fabricated zero"),
the `-1` the broker itself sends for an undeterminable gRPC lag,
`onlineInstances = -1`, and the
reset-offset preview's own `minOffset/maxOffset = -1`, which the console
already renders as
unavailable through `formatOffsetValue` (`consumer.tsx:185-186`) and the
`targetOffset < 0 ||
consumerOffset < 0` branch at `:228`.
### Fix
- `QueueProgressVO` gains `UNKNOWN_OFFSET = -1L` so the providers and the
console have one name
for the sentinel.
- Both cloud providers report it for `brokerOffset`/`consumerOffset`;
`diffTotal` (the real,
per-topic lag) is untouched.
- The progress table renders an offset through the existing
`formatOffsetValue`, so a negative
offset shows as `-` instead of a number (the same helper the reset preview
already uses for the
same quantity).
The AI `rmq.group.detail` contract keeps receiving numbers for these fields,
so its required
output schema is unaffected (the same trap that makes a nullable offset
unrepresentable there).
### Evidence (pre-fix, on the base commit `1ef5d860`; new tests kept, only
the two provider files reverted so they compile)
```
$ cd server && mvn -o test
-Dtest='AliyunConvertersLagTest,TencentInstanceProviderTest'
[ERROR]
org.apache.rocketmq.studio.provider.tencent.TencentInstanceProviderTest.getGroupProgressShouldReportUnknownQueueOffsetsTest
<<< FAILURE!
expected: -1L
but was: 0L
[ERROR]
org.apache.rocketmq.studio.provider.alibaba.AliyunConvertersLagTest.queueOffsetsShouldBeUnknownWhenOnlyTheTopicLagIsKnownTest
<<< FAILURE!
expected: -1L
but was: 0L
[ERROR]
org.apache.rocketmq.studio.provider.alibaba.AliyunConvertersLagTest.aggregateRowOffsetsShouldBeUnknownTooTest
<<< FAILURE!
expected: -1L
but was: 0L
Tests run: 55, Failures: 3, Errors: 0, Skipped: 0
```
```
$ cd web && npx vitest run
src/pages/instance/__tests__/ConsumerPage.test.tsx -t "renders an offset the
provider cannot report as unavailable"
× renders an offset the provider cannot report as unavailable
AssertionError: expected [] to have a length of 2 but got +0 // no cell
reads as unavailable
Tests 1 failed | 36 skipped (37)
```
The three Java cases pin the fabricated zero on the wire, and the component
case pins that the
sentinel is rendered as unavailable rather than as a number — together they
cover the row an
operator actually sees.
### Duplicate check
Searched issues and PRs (open and closed) for `queue progress`, `consumer
progress`,
`brokerOffset`, `consumerOffset`, `QueueProgressVO`, `unavailable` and read
every open PR touching
the two provider files: #4884 (subscription consistency), #4857 (Aliyun body
encoding), #4844
(lite topic type), #4732 (subscription filter modes), #4514 (pagination) —
none of them changes the
progress rows. #4517 (merged) fixed the *lag* of the Apache provider,
#4674/#4675 (open, other
contributors) make lag samples and the consumer delay unavailable in the
metrics collectors; all of
them leave these two offsets as they are.
Out of scope, deliberately: the pseudo-broker name (`topic:<topic>`) and the
`queueId = 0` of these
aggregate rows, and the summary cards that count rows as queues. Changing
what those columns mean
is a presentation decision, not a fabricated measurement.
### Scoring (AGENTS.md)
`PRIORITY` = impact 20 + reach 12 + reproducibility 18 + maintenance value
14 = **64**;
`FIX_CONFIDENCE` = **85**. (Impact: monitoring data that contradicts itself
in the same row for
every cloud group, and an offset of zero feeding the reset preview; reach:
the progress tab of
every Tencent/Aliyun consumer group; reproducibility: three deterministic
provider tests plus one
component test.)
### How Did You Test This Change?
```
$ cd server && mvn -o test
-Dtest='AliyunConvertersLagTest,TencentInstanceProviderTest'
Tests run: 55, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
$ cd server && mvn -o test
-Dtest='ConsumerGroupControllerTest,MetadataServiceTest,ConsumerGroupReadToolHandlersTest,InstanceProviderTest,ApacheInstanceProviderTest,AliyunInstanceProviderTest,TencentInstanceProviderTest,AliyunConvertersTest,AliyunConvertersLagTest,CloudRocketMqBusinessMetricsCollectorTest,ApacheRocketMqBusinessMetricsCollectorTest,RocketMQMetadataProviderTest'
Tests run: 238, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
$ cd server && mvn -o test
Tests run: 3153, Failures: 0, Errors: 17, Skipped: 0
```
The 17 errors are the pre-existing `ApplicationContext` load failures of the
MySQL-backed
integration tests (`Communications link failure`; this checkout has no
MySQL) — the same count as
on the base commit, none of them in the provider or group packages.
```
$ cd web && npx vitest run src/pages/instance/__tests__/ConsumerPage.test.tsx
Test Files 1 passed (1)
Tests 37 passed (37)
$ cd web && npx vitest run src/pages/instance src/components
Test Files 16 passed (16)
Tests 261 passed (261)
$ cd web && npx tsc -b && npx eslint src/pages/instance/consumer.tsx
src/pages/instance/__tests__/ConsumerPage.test.tsx
(no output, exit 0)
```
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [x] Tests added for the changed behaviour
- [x] New UI text: none (the `-` comes from the existing `formatOffsetValue`)
- [x] Architecture constraints stay green (`mvn test` ran the ArchUnit
checks)
- [x] New source files: none
- [x] Documentation: not touched
### Risk
Low, and one-directional: the change replaces a fabricated value with an
explicit "unavailable".
`diffTotal` — the only field the lag metrics, the group-detail tool and the
aggregate cards read —
is unchanged, and the Apache provider's real offsets are untouched.
Consumers of the JSON that
assumed a non-negative offset now see `-1`, which is the same sentinel those
paths already handle
for an undeterminable lag.
--
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]
