diff --git a/proxy/src/main/java/org/apache/rocketmq/proxy/grpc/v2/channel/GrpcClientChannel.java b/proxy/src/main/java/org/apache/rocketmq/proxy/grpc/v2/channel/GrpcClientChannel.java index 0135818fb3b..b6c08b9f02c 100644 --- a/proxy/src/main/java/org/apache/rocketmq/proxy/grpc/v2/channel/GrpcClientChannel.java +++ b/proxy/src/main/java/org/apache/rocketmq/proxy/grpc/v2/channel/GrpcClientChannel.java @@ -25,7 +25,6 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.ComparisonChain; import com.google.protobuf.InvalidProtocolBufferException; -import com.google.protobuf.TextFormat; import com.google.protobuf.util.JsonFormat; import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; @@ -263,24 +262,59 @@ public String getClientId() { public void writeTelemetryCommand(TelemetryCommand command) { StreamObserver observer = this.telemetryCommandRef.get(); if (observer == null) { - log.warn("telemetry command observer is null when try to write data. command:{}, channel:{}", TextFormat.shortDebugString(command), this); + log.warn("telemetry command observer is null when try to write data. command:{}, channel:{}", + summarizeTelemetryCommand(command), this); return; } synchronized (this.telemetryWriteLock) { observer = this.telemetryCommandRef.get(); if (observer == null) { - log.warn("telemetry command observer is null when try to write data. command:{}, channel:{}", TextFormat.shortDebugString(command), this); + log.warn("telemetry command observer is null when try to write data. command:{}, channel:{}", + summarizeTelemetryCommand(command), this); return; } try { observer.onNext(command); } catch (StatusRuntimeException | IllegalStateException exception) { - log.warn("write telemetry failed. command:{}", command, exception); + log.warn("write telemetry failed. command:{}, channel:{}", summarizeTelemetryCommand(command), this, exception); this.clearClientObserver(observer); } } } + static String summarizeTelemetryCommand(TelemetryCommand command) { + if (command == null) { + return "null"; + } + + StringBuilder builder = new StringBuilder(command.getCommandCase().name()); + switch (command.getCommandCase()) { + case PRINT_THREAD_STACK_TRACE_COMMAND: + builder.append(", nonce=").append(command.getPrintThreadStackTraceCommand().getNonce()); + break; + case VERIFY_MESSAGE_COMMAND: + builder.append(", nonce=").append(command.getVerifyMessageCommand().getNonce()); + break; + case RECOVER_ORPHANED_TRANSACTION_COMMAND: + builder.append(", transactionId=") + .append(command.getRecoverOrphanedTransactionCommand().getTransactionId()) + .append(", details omitted"); + break; + case NOTIFY_UNSUBSCRIBE_LITE_COMMAND: + builder.append(", liteTopic=") + .append(command.getNotifyUnsubscribeLiteCommand().getLiteTopic()); + break; + case SETTINGS: + builder.append(", clientType=").append(command.getSettings().getClientType()) + .append(", pubSubCase=").append(command.getSettings().getPubSubCase()); + break; + default: + // Add explicit cases for future commands that carry sensitive payloads. + break; + } + return builder.toString(); + } + @Override public String toString() { return MoreObjects.toStringHelper(this) diff --git a/proxy/src/test/java/org/apache/rocketmq/proxy/grpc/v2/channel/GrpcClientChannelTest.java b/proxy/src/test/java/org/apache/rocketmq/proxy/grpc/v2/channel/GrpcClientChannelTest.java index 1bdbdd9befe..369ebdbaf19 100644 --- a/proxy/src/test/java/org/apache/rocketmq/proxy/grpc/v2/channel/GrpcClientChannelTest.java +++ b/proxy/src/test/java/org/apache/rocketmq/proxy/grpc/v2/channel/GrpcClientChannelTest.java @@ -17,9 +17,16 @@ package org.apache.rocketmq.proxy.grpc.v2.channel; +import apache.rocketmq.v2.Message; +import apache.rocketmq.v2.NotifyUnsubscribeLiteCommand; import apache.rocketmq.v2.Publishing; +import apache.rocketmq.v2.PrintThreadStackTraceCommand; +import apache.rocketmq.v2.RecoverOrphanedTransactionCommand; import apache.rocketmq.v2.Resource; import apache.rocketmq.v2.Settings; +import apache.rocketmq.v2.TelemetryCommand; +import apache.rocketmq.v2.VerifyMessageCommand; +import com.google.protobuf.ByteString; import org.apache.commons.lang3.RandomStringUtils; import org.apache.rocketmq.proxy.common.ProxyContext; import org.apache.rocketmq.proxy.config.InitConfigTest; @@ -35,7 +42,9 @@ import org.mockito.junit.MockitoJUnitRunner; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -79,4 +88,79 @@ public void testChannelExtendAttributeParse() { assertEquals(clientSettings, GrpcClientChannel.parseChannelExtendAttribute(this.grpcClientChannel)); assertNull(GrpcClientChannel.parseChannelExtendAttribute(mock(RemotingChannel.class))); } -} \ No newline at end of file + + @Test + public void testSummarizeTelemetryCommandDoesNotIncludeMessagePayload() { + TelemetryCommand command = TelemetryCommand.newBuilder() + .setVerifyMessageCommand(VerifyMessageCommand.newBuilder() + .setNonce("nonce-1") + .setMessage(Message.newBuilder() + .setBody(ByteString.copyFromUtf8("secret-body")) + .build()) + .build()) + .build(); + + String summary = GrpcClientChannel.summarizeTelemetryCommand(command); + + assertTrue(summary.contains("VERIFY_MESSAGE_COMMAND")); + assertTrue(summary.contains("nonce-1")); + assertFalse(summary.contains("secret-body")); + assertFalse(summary.contains("message")); + assertFalse(summary.contains("body")); + } + + @Test + public void testSummarizeRecoverTransactionCommandDoesNotIncludeMessagePayload() { + TelemetryCommand command = TelemetryCommand.newBuilder() + .setRecoverOrphanedTransactionCommand(RecoverOrphanedTransactionCommand.newBuilder() + .setTransactionId("transaction-id") + .setMessage(Message.newBuilder() + .setBody(ByteString.copyFromUtf8("secret-body")) + .build()) + .build()) + .build(); + + String summary = GrpcClientChannel.summarizeTelemetryCommand(command); + + assertTrue(summary.contains("RECOVER_ORPHANED_TRANSACTION_COMMAND")); + assertTrue(summary.contains("transaction-id")); + assertTrue(summary.contains("details omitted")); + assertFalse(summary.contains("secret-body")); + assertFalse(summary.contains("message")); + assertFalse(summary.contains("body")); + } + + @Test + public void testSummarizeTelemetryCommandDiagnosticFields() { + assertEquals("null", GrpcClientChannel.summarizeTelemetryCommand(null)); + assertEquals("COMMAND_NOT_SET", GrpcClientChannel.summarizeTelemetryCommand(TelemetryCommand.getDefaultInstance())); + + TelemetryCommand settingsCommand = TelemetryCommand.newBuilder() + .setSettings(Settings.newBuilder() + .setPublishing(Publishing.getDefaultInstance()) + .build()) + .build(); + String settingsSummary = GrpcClientChannel.summarizeTelemetryCommand(settingsCommand); + assertTrue(settingsSummary.contains("SETTINGS")); + assertTrue(settingsSummary.contains("clientType=")); + assertTrue(settingsSummary.contains("pubSubCase=PUBLISHING")); + + TelemetryCommand threadStackCommand = TelemetryCommand.newBuilder() + .setPrintThreadStackTraceCommand(PrintThreadStackTraceCommand.newBuilder() + .setNonce("stack-nonce") + .build()) + .build(); + String threadStackSummary = GrpcClientChannel.summarizeTelemetryCommand(threadStackCommand); + assertTrue(threadStackSummary.contains("PRINT_THREAD_STACK_TRACE_COMMAND")); + assertTrue(threadStackSummary.contains("stack-nonce")); + + TelemetryCommand liteCommand = TelemetryCommand.newBuilder() + .setNotifyUnsubscribeLiteCommand(NotifyUnsubscribeLiteCommand.newBuilder() + .setLiteTopic("lite-topic") + .build()) + .build(); + String liteSummary = GrpcClientChannel.summarizeTelemetryCommand(liteCommand); + assertTrue(liteSummary.contains("NOTIFY_UNSUBSCRIBE_LITE_COMMAND")); + assertTrue(liteSummary.contains("lite-topic")); + } +}