unbridled-41 opened a new pull request, #5120:
URL: https://github.com/apache/rocketmq-dashboard/pull/5120
### Issue linkage
No existing issue or open PR covers this gap. `gh search prs/issues --repo
apache/rocketmq-dashboard "executeWithAudit"` returns nothing, and a full scan
of the 211 open PR bodies and 217 open issues for `executeWithAudit` /
`SEND_MESSAGE` / `cloud send audit` finds only PRs about topic send policies
(#4847), the redelivery payload (#4737) and the DLQ resend outcome (#4592). The
open cloud-audit items #4656/#4660 are about import audit grading.
### Problem / Evidence
`MetadataService` wraps every cloud-capable mutation in `executeWithAudit`
(`MetadataService.java:866-888`), which deliberately skips Apache because the
Apache admin client records the row itself. `sendMessage` is the one write that
never used the wrapper:
```java
public SendMessageVO sendMessage(SendMessageDTO request) {
requireSendMessageRequest(request);
request.setTopic(requireName(request.getTopic(), "topicName"));
request.setInstanceId(requireWriteInstance(request.getInstanceId(),
ownershipGuard.topicResource(request.getTopic()), true));
return resolve(request.getInstanceId()).sendMessage(request); // <- no
audit
}
```
- Apache: `RocketMQAdminClientImpl.sendMessage` records `SEND_MESSAGE` for
both outcomes (`:563`, `:575`, `:578`, `:588`, resource type `MESSAGE`).
- Aliyun / Tencent: `AliyunInstanceProvider.sendMessage` (`:478-499`) and
`TencentInstanceProvider.sendMessage` (`:590-611`) contain no audit call at all.
- Sibling writes (`createTopic`, `updateTopic`, `deleteTopic`, group
create/update/delete, `resetOffset`, both import loops) all route through the
wrapper, so this is the one hole in the same mechanism.
Call path: `TopicController.sendMessage:125-129` ->
`MetadataService.sendMessage:293` -> `resolve(instanceId).sendMessage(request)`
-> cloud provider. The same method also serves the AI tool
(`MessageSendToolHandler` -> `metadataService.sendMessage`) and message
redelivery (`redeliverMessage:341`).
Observed **before** the fix — the two new tests, run against the pre-fix
revision of `MetadataService` (the constants and tests kept in place):
```text
Wanted but not invoked:
operationAuditService.record("SEND_MESSAGE", "MESSAGE", "orders",
"cloud-instance", ..., "SUCCESS", null);
Actually, there were zero interactions with this mock.
Tests run: 2, Failures: 2, Errors: 0
```
So a test message sent to a Tencent/Aliyun instance — and the same send
performed by the `rmq.message.send` AI tool or by message redelivery on a cloud
instance — left no trace in `/api/audit`, neither on success nor on failure,
while the identical action on an Apache instance is recorded twice (never: the
Apache path records once, and the wrapper skips it).
Observed **after** the fix: both tests pass; the row is written with the
same operation (`SEND_MESSAGE`), resource type (`MESSAGE`) and resource name
(the topic) the Apache path uses, `SUCCESS` on completion and `FAILED` with the
failure message otherwise.
### Root cause / Fix
The send path was never migrated to the shared audit wrapper when the cloud
providers landed. Route it through `executeWithAudit` like its siblings, which
needs the two audit-vocabulary constants (`Operation.SEND_MESSAGE`,
`ResourceType.MESSAGE`) the Apache path writes as literals.
### Priority / fix confidence
- PRIORITY = **64** — impact 26/40 (a mutating operation on a cloud instance
leaves no audit trail at all, on a console whose audit log is a first-class
feature), scope 16/20 (every cloud send, including the AI tool and cloud
redelivery), reproducibility 16/20 (deterministic unit test), maintenance value
6/20 (closes the one hole in an existing mechanism).
- FIX_CONFIDENCE = **86** — the wrapper, its Apache skip and the exact
operation/resource vocabulary already exist and are pinned by
`apacheTopicWriteOperationsShouldNotDuplicateProviderAudit`.
### Tests
| Command | Result |
|---|---|
| `mvn -o -q test -Dtest=MetadataServiceTest` | 52 tests, 0 failures |
| `mvn -o -q test
-Dtest='MetadataServiceTest,TopicControllerTest,MessageSendToolHandlerTest,MessageRedeliveryToolHandlerTest,org.apache.rocketmq.studio.audit.**'`
| 93 tests, 0 failures, 0 errors |
| `mvn -o test` (whole suite) | Tests run: 3289, Failures: 0, Errors: 17 |
| `mvn -o checkstyle:check` | clean |
The 17 whole-suite errors are the MySQL-backed integration tests that cannot
load a Spring context in this environment; the errored set is identical to the
one measured on the untouched base revision `4ee173ad`.
New tests:
`MetadataServiceTest#cloudSendShouldBeAuditedLikeTheApachePathTest` and
`#failedCloudSendShouldBeAuditedWithItsReasonTest` (both fail pre-fix as shown
above). `apacheTopicWriteOperationsShouldNotDuplicateProviderAudit` (which also
sends a message) still pins that the Apache path writes no second row.
### Risk
Cloud sends now write one audit row per attempt, including attempts made
through `redeliverMessage` (the AI `rmq.message.redeliver` tool and the message
page's redelivery action) and failures. That is the behaviour the Apache path
already has, but it does add rows to existing cloud deployments' audit feeds. A
`redeliverMessage` on a cloud instance is recorded as `SEND_MESSAGE` (its
destination topic) because no separate redelivery operation exists in this
path; if the maintainers want it distinguished, that is a follow-up rather than
part of this fix.
--
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]
