wang-jiahua commented on code in PR #10539:
URL: https://github.com/apache/rocketmq/pull/10539#discussion_r3499693647
##########
remoting/src/main/java/org/apache/rocketmq/remoting/metrics/RemotingMetricsManager.java:
##########
@@ -87,6 +95,37 @@ public List<Pair<InstrumentSelector, ViewBuilder>>
getMetricsView() {
return Lists.newArrayList(new Pair<>(selector, viewBuilder));
}
+ public Attributes getOrBuildAttributes(int requestCode, int responseCode,
+ boolean isLongPolling, String result) {
+ int resultIdx;
+ if (RESULT_SUCCESS.equals(result)) resultIdx = 0;
+ else if (RESULT_ONEWAY.equals(result)) resultIdx = 1;
+ else if (RESULT_WRITE_CHANNEL_FAILED.equals(result)) resultIdx = 2;
+ else if (RESULT_CANCELED.equals(result)) resultIdx = 3;
+ else resultIdx = -1;
+
+ if (resultIdx < 0) {
+ return buildAttributes(requestCode, responseCode, isLongPolling,
result);
+ }
+
+ long key = ((long) requestCode << 19)
Review Comment:
Added `RemotingMetricsManagerTest` with 7 test cases in the latest push:
- `testCacheHitSameArgs`: same args return cached instance
- `testDifferentRequestCode` / `testDifferentResponseCode` /
`testDifferentIsLongPolling` / `testDifferentResult`: each parameter change
produces distinct cached Attributes
- `testUnknownResultNotCached`: unknown result values use the non-cached
path (new instance each time)
- `testKnownResultsCached`: all 4 known result types (SUCCESS, ONEWAY,
WRITE_CHANNEL_FAILED, CANCELED) are properly cached
##########
remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingAbstract.java:
##########
@@ -235,19 +234,14 @@ public static void writeResponse(Channel channel,
RemotingCommand request, @Null
if (response == null) {
return;
}
- final AttributesBuilder attributesBuilder;
- if (remotingMetricsManager != null) {
- attributesBuilder = remotingMetricsManager.newAttributesBuilder();
- attributesBuilder.put(LABEL_IS_LONG_POLLING, request.isSuspended())
- .put(LABEL_REQUEST_CODE,
RemotingHelper.getRequestCodeDesc(request.getCode()))
- .put(LABEL_RESPONSE_CODE,
RemotingHelper.getResponseCodeDesc(response.getCode()));
- } else {
- attributesBuilder = null;
- }
+ final int requestCode = request.getCode();
+ final int responseCode = response.getCode();
+ final boolean isLongPolling = request.isSuspended();
if (request.isOnewayRPC()) {
- if (attributesBuilder != null) {
- attributesBuilder.put(LABEL_RESULT, RESULT_ONEWAY);
-
remotingMetricsManager.getRpcLatency().record(request.getProcessTimer().elapsed(TimeUnit.MILLISECONDS),
attributesBuilder.build());
+ if (remotingMetricsManager != null) {
+ Attributes attrs = remotingMetricsManager.getOrBuildAttributes(
+ requestCode, responseCode, isLongPolling, RESULT_ONEWAY);
+
remotingMetricsManager.getRpcLatency().record(request.getProcessTimer().elapsed(TimeUnit.MILLISECONDS),
attrs);
Review Comment:
Updated the PR description — removed the `processTimerElapsedMs` mention.
This PR only adds the Attributes cache and the `isDebugEnabled()` guard. The
timing change is not part of this PR.
--
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]
