[ISSUE #10441] Reduce per-RPC allocation in metrics by caching static AttributeKey instances

[wang-jiahua]

Which Issue(s) This PR Fixes

Fixes #10441

Brief Description

Reduce per-RPC allocation by replacing reflective AttributeKey.stringKey(name) / AttributeKey.booleanKey(name) calls with pre-built static AttributeKey<T> singletons.

JFR heap dump showed 38,182 InternalAttributeKeyImpl instances for only 6 distinct key names. Every AttributesBuilder.put(String, T) call internally creates a fresh AttributeKey — a stateless immutable object that should be a singleton.

Define static final AttributeKey<T> LABEL_*_KEY constants in BrokerMetricsConstant and RemotingMetricsConstant, then migrate all put(String, T) callsites to put(AttributeKey<T>, T).

Three misc optimizations previously bundled here (NopStatusListener, TopicMessageType.metricsValue cache, RemotingHelper eager eval fix) have been split into PR #10491.

How Did You Test This Change?

1. Unit tests pass (maven-compile on all JDK versions: 8, 11, 17, 21).
2. Master CI passes. Bazel compile failure is a known flaky integration test (NormalMsgTwoSameGroupConsumerIT), unrelated to this PR.
3. Commercial compatibility verified — no commercial classes are affected.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR focuses on reducing allocation/CPU overhead on metrics and remoting hot paths by introducing AttributeKey singletons and adding small caches for frequently repeated attribute/metric lookups.

Changes:

• Added fast-path caching for remoting request/response code distribution counting.
• Introduced typed OpenTelemetry AttributeKey constants and attribute caching/build helpers in remoting and broker metrics.
• Reduced repeated string allocations (e.g., cached lowercase metrics value, avoided Map.getOrDefault boxing paths).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
remoting/src/main/java/org/apache/rocketmq/remoting/netty/RemotingCodeDistributionHandler.java	Adds “last code” fast path to reduce map lookups for hot request/response codes.
remoting/src/main/java/org/apache/rocketmq/remoting/metrics/RemotingMetricsManager.java	Adds attribute caching and typed label keys to reduce OpenTelemetry attribute overhead.
remoting/src/main/java/org/apache/rocketmq/remoting/metrics/RemotingMetricsConstant.java	Introduces typed AttributeKey singletons for labels.
remoting/src/main/java/org/apache/rocketmq/remoting/common/RemotingHelper.java	Minor optimization to avoid getOrDefault overhead for code descriptions.
common/src/main/java/org/apache/rocketmq/common/attribute/TopicMessageType.java	Caches lowercase metrics value to avoid repeated conversions.
broker/src/main/resources/rmq.broker.logback.xml	Suppresses Logback status output via NopStatusListener.
broker/src/main/java/org/apache/rocketmq/broker/metrics/PopMetricsManager.java	Switches to typed label keys for attributes.
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsManager.java	Adds topic attributes cache and switches to typed label keys.
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsConstant.java	Introduces typed AttributeKey singletons for broker metrics labels.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 31.37255% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.11%. Comparing base (59a70d9) to head (393393a).

Files with missing lines	Patch %	Lines
.../rocketmq/broker/metrics/BrokerMetricsManager.java	0.00%	25 Missing ⚠️
...etmq/remoting/metrics/RemotingMetricsConstant.java	0.00%	5 Missing ⚠️
...che/rocketmq/broker/metrics/PopMetricsManager.java	33.33%	4 Missing ⚠️
...ketmq/remoting/metrics/RemotingMetricsManager.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10443      +/-   ##
=============================================
- Coverage      48.13%   48.11%   -0.03%     
+ Complexity     13379    13375       -4     
=============================================
  Files           1377     1377              
  Lines         100730   100749      +19     
  Branches       13012    13012              
=============================================
- Hits           48491    48480      -11     
- Misses         46300    46319      +19     
- Partials        5939     5950      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[oss-sentinel-ai]

Review by github-manager-bot

Summary

This PR caches OpenTelemetry AttributeKey instances as static finals to avoid per-RPC allocation, optimizes TopicMessageType lookup, adds a volatile inline cache for RemotingCodeDistributionHandler, and adds Logback NopStatusListener to suppress startup log noise.

Findings

• [Good] BrokerMetricsConstant.java:69-86 — Pre-built typed AttributeKey singletons eliminate per-call allocation on the metrics hot path.
• [Good] BrokerMetricsManager.java:105-120 — Import changes correctly reference the new cached keys.
• [Good] RemotingCodeDistributionHandler.java:35-68 — Volatile inline cache pattern correctly avoids ConcurrentHashMap.computeIfAbsent() on repeated request codes.
• [Good] TopicMessageType.java — Enum-based lookup is more efficient than string comparison.
• [Good] rmq.broker.logback.xml — NopStatusListener suppresses Logback startup noise in production.

Suggestions

1. The volatile inline cache pattern in RemotingCodeDistributionHandler is effective for sequential workloads. Consider documenting that this optimization assumes temporal locality in request codes.
2. Consider adding a microbenchmark to quantify the allocation reduction.

Verdict

✅ Approved — Solid performance optimization with clean implementation.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Review by github-manager-bot

Summary

Pre-builds AttributeKey singletons as static constants in BrokerMetricsConstant and updates all call sites to use them, eliminating per-RPC allocation of InternalAttributeKeyImpl objects.

Findings

• [Critical] BrokerMetricsConstant.java:69-83 — All 15 AttributeKey singletons are correctly typed (stringKey for String labels, booleanKey for boolean labels). Each call to AttributeKey.stringKey("name") creates a new object; caching eliminates this per-RPC allocation.
• [Critical] BrokerMetricsManager.java, PopMetricsManager.java, RemotingMetricsManager.java — All call sites consistently updated to use the cached keys. No remaining AttributeKey.stringKey() calls on hot paths.
• [Info] RemotingMetricsManager.java:125-135 — The newAttributes() helper now uses cached keys. This is called on every RPC completion, so the savings are proportional to RPC throughput.

Suggestions

No concerns. This is a straightforward, low-risk optimization with clear benefits on high-throughput brokers.

Verdict: LGTM. Clean optimization with consistent application across all metrics call sites.

---

Automated review by github-manager-bot

[RongtongJin]

LGTM

[RockteMQ-AI]

Review by github-manager-bot

Summary

Introduces pre-built static final AttributeKey<T> singletons in BrokerMetricsConstant and RemotingMetricsConstant, then replaces all AttributesBuilder.put(String, T) calls with typed put(AttributeKey<T>, T) overloads in BrokerMetricsManager, PopMetricsManager, and RemotingMetricsManager to eliminate per-RPC InternalAttributeKeyImpl allocations.

Findings

• [Info] JFR data cited in the issue (38,182 instances for 6 distinct keys) makes a strong case. The singleton pattern is the correct fix.
• [Info] All call sites consistently use the typed put(AttributeKey<T>, T) overload — no mixed usage detected.
• [Info] The LABEL_INVOCATION_STATUS_KEY uses AttributeKey.stringKey() which is correct since InvocationStatus is stored as its .name() String.
• [Warning] PopMetricsManager uses LABEL_CONSUMER_GROUP_KEY for the group label — ensure this is the same semantic as the one in BrokerMetricsConstant (it is, but worth a comment for future maintainers).

Suggestions

• Minor: consider grouping the LABEL_*_KEY constants with a comment block explaining they are hot-path singletons, so future contributors don't accidentally remove "unused" constants.

No blocking issues. Solid, well-targeted optimization.

---

Automated review by github-manager-bot