RockteMQ-AI commented on PR #1368: URL: https://github.com/apache/rocketmq-clients/pull/1368#issuecomment-5614910552
### Summary Follow-up review for 4 new commits pushed after the previous review (2026-09-09T13:36Z): - `feat(nodejs): add metrics export pipeline, custom IpNameResolver` - `fix(nodejs): defer logger close so subclass shutdown logs do not throw` - `test(nodejs): use a fixed consumer group in the simple consumer integration test` - `fix(nodejs): correct a codespell finding in the IpNameResolver test` These commits add **OpenTelemetry metrics export**, a **custom `ip:` gRPC name resolver**, a **counting Semaphore** for in-flight message throttling, and a **PushConsumerGaugeObserver** — all mirroring the Java client's architecture. ### Findings - **[Warning]** `Semaphore.ts` — `release()` does not cap permits at the initial count. If called more times than `acquire()`, permits grow unbounded. Consider adding an upper bound or documenting that callers must not release more than they acquire. - **[Info]** `IpNameResolver.ts` — The Fisher-Yates shuffle for endpoint randomization is correct. The `ip:` scheme parsing handles both IPv4 and IPv6 (bracket notation) properly. - **[Info]** `ClientMeterManager.ts` — Clean OpenTelemetry integration. The `GaugeObserver` interface allows pluggable gauge sources, matching the Java `GaugeObserver` SPI. - **[Info]** `PushConsumerGaugeObserver.ts` — Correctly aggregates per-`ProcessQueue` cache stats by topic. The intermediate `Map<string, number>` accumulator before building the final `Map<Attributes, number>` is efficient. - **[Info]** Tests: `Semaphore.test.ts`, `IpNameResolver.test.ts`, `metrics.test.ts`, and `defect-fixes.test.ts` provide good coverage of the new functionality. ### Suggestions The new features are well-structured and closely follow the Java client patterns. The only actionable item is the Semaphore overflow guard. Otherwise, LGTM for the incremental changes. --- *Automated review by github-manager* -- 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]
