Copilot commented on code in PR #10491:
URL: https://github.com/apache/rocketmq/pull/10491#discussion_r3401933381
##########
broker/src/main/resources/rmq.broker.logback.xml:
##########
@@ -18,6 +18,8 @@
<configuration scan="true" scanPeriod="30 seconds">
+ <statusListener class="ch.qos.logback.core.status.NopStatusListener"/>
Review Comment:
Adding `NopStatusListener` suppresses Logback status output globally, which
can hide important configuration/startup problems (e.g., misconfigured
appenders, missing files) and make production incidents harder to debug. If the
intent is to reduce noise, consider gating this behind a profile/system
property or using a listener that still surfaces WARN/ERROR status messages
rather than dropping everything.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/metrics/RemotingMetricsConstant.java:
##########
@@ -24,6 +26,14 @@ public class RemotingMetricsConstant {
public static final String LABEL_IS_LONG_POLLING = "is_long_polling";
public static final String LABEL_RESULT = "result";
+ // Pre-built typed AttributeKey singletons. Use these in
AttributesBuilder.put()
+ // on hot paths to avoid allocating a fresh InternalAttributeKeyImpl per
call.
Review Comment:
The comment references `InternalAttributeKeyImpl`, which is an OpenTelemetry
internal implementation detail and may change across versions, making the
comment potentially misleading/outdated. Consider rewording to describe the
observable behavior (e.g., avoiding repeated `AttributeKey.*Key(...)`
construction / reducing allocations) without naming internal classes.
##########
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsConstant.java:
##########
@@ -64,4 +66,21 @@ public class BrokerMetricsConstant {
public static final String LABEL_LANGUAGE = "language";
public static final String LABEL_VERSION = "version";
public static final String LABEL_CONSUME_MODE = "consume_mode";
+
+ // Pre-built typed AttributeKey singletons. Use these in
AttributesBuilder.put()
+ // on hot paths to avoid allocating a fresh InternalAttributeKeyImpl per
call.
Review Comment:
Same issue as in remoting constants: referencing an OpenTelemetry internal
class name in a public comment is brittle. Suggest updating the wording to
avoid internal implementation details and just state the intent (reuse typed
`AttributeKey` constants to reduce repeated key creation/allocation).
--
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]
