wang-jiahua commented on code in PR #10487:
URL: https://github.com/apache/rocketmq/pull/10487#discussion_r3400423138
##########
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsManager.java:
##########
@@ -229,7 +229,10 @@ public static boolean isSystem(String topic, String group)
{
}
public static TopicMessageType getMessageType(SendMessageRequestHeader
requestHeader) {
- Map<String, String> properties =
MessageDecoder.string2messageProperties(requestHeader.getProperties());
+ return
getMessageType(MessageDecoder.string2messageProperties(requestHeader.getProperties()));
+ }
+
+ public static TopicMessageType getMessageType(Map<String, String>
properties) {
Review Comment:
It must be `public` because the downstream caller is `SendMessageProcessor`
in the `broker` module — a different package
(`org.apache.rocketmq.broker.processor`) from `BrokerMetricsManager`
(`org.apache.rocketmq.broker.metrics`). Package-private would not be visible
across packages. The overload is intentionally public to allow any processor or
hook to call it with an already-decoded Map.
##########
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsManager.java:
##########
@@ -229,7 +229,10 @@ public static boolean isSystem(String topic, String group)
{
}
public static TopicMessageType getMessageType(SendMessageRequestHeader
requestHeader) {
- Map<String, String> properties =
MessageDecoder.string2messageProperties(requestHeader.getProperties());
+ return
getMessageType(MessageDecoder.string2messageProperties(requestHeader.getProperties()));
+ }
+
+ public static TopicMessageType getMessageType(Map<String, String>
properties) {
String traFlag =
properties.get(MessageConst.PROPERTY_TRANSACTION_PREPARED);
Review Comment:
Good point. The existing `getMessageType(SendMessageRequestHeader)` also
doesn't guard against null `requestHeader.getProperties()` (it would NPE inside
`string2messageProperties`). For consistency with the existing contract, the
new overload follows the same pattern. That said, I can add a null guard
returning `NORMAL` if you prefer — let me know.
--
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]
