This is an automated email from the ASF dual-hosted git repository. dmsolr pushed a commit to branch perf/issue-7837-websocket-collector-redaction in repository https://gitbox.apache.org/repos/asf/shenyu.git
commit b9dbed076a2ed68253a6de9b06deccb2fd03d21f Author: Luke Haochao Zhuang <[email protected]> AuthorDate: Mon Oct 19 15:14:49 2026 +0800 perf(admin): avoid full-payload redaction in the websocket send log (#7837) WebsocketCollector.send(namespaceId, message, type) passed maskSensitive(message) as an INFO argument. Java evaluates arguments before the logger checks its level, so on every config push the whole message was parsed into maps/lists, recursively traversed and re-serialized into a second complete JSON string -- even when INFO was disabled, and before the send path discovered whether any matching sessions existed. Guard the diagnostic by level and stop serializing the payload for it: - INFO now logs bounded delivery metadata (namespaceId, eventType, size in chars) rather than the entire message. The group/eventType/item count are already logged by the publisher (WebsocketDataChangedListener), so this reuses available metadata instead of reparsing a large JSON to recover it. - The full payload stays available as an opt-in DEBUG diagnostic, still redacted through maskSensitive and now lazy/level-guarded, so the unredacted message is never logged. - Validation (blank message/namespace) and delivery are unchanged and run regardless of the log level. A test attaches a Logback ListAppender to the root logger and asserts an ordinary send emits exactly one bounded INFO diagnostic carrying the namespace/eventType/size and none of the payload shape, secret or redaction marker. maskSensitive redaction stays covered by testNestedApiKeysAreRedacted. No before/after load benchmark has been run; per the issue the caller-thread CPU and allocation, log bytes/sec and p99 measurements are left to the benchmark contract in #7650. This is the deterministic, source-grounded part of #7837. --- .../listener/websocket/WebsocketCollector.java | 11 +++++- .../listener/websocket/WebsocketCollectorTest.java | 45 ++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/shenyu-admin/src/main/java/org/apache/shenyu/admin/listener/websocket/WebsocketCollector.java b/shenyu-admin/src/main/java/org/apache/shenyu/admin/listener/websocket/WebsocketCollector.java index 4169242baa..cb9437e4ae 100644 --- a/shenyu-admin/src/main/java/org/apache/shenyu/admin/listener/websocket/WebsocketCollector.java +++ b/shenyu-admin/src/main/java/org/apache/shenyu/admin/listener/websocket/WebsocketCollector.java @@ -322,7 +322,16 @@ public class WebsocketCollector { if (StringUtils.isBlank(namespaceId)) { throw new ShenyuException("namespaceId can not be null"); } - LOG.info("websocket send message to namespaceId: {}, message: {}", namespaceId, maskSensitive(message)); + if (LOG.isInfoEnabled()) { + // Log bounded delivery metadata instead of reparsing, redacting and re-serializing the whole + // payload on the publisher thread. The group/eventType/item count are already logged by the + // publisher (WebsocketDataChangedListener); the full redacted payload stays opt-in at DEBUG. + LOG.info("websocket send message to namespaceId: {}, eventType: {}, size: {} chars", + namespaceId, type, message.length()); + } + if (LOG.isDebugEnabled()) { + LOG.debug("websocket send message to namespaceId: {}, payload: {}", namespaceId, maskSensitive(message)); + } if (DataEventTypeEnum.MYSELF == type) { Session session = (Session) ThreadLocalUtils.get(SESSION_KEY); if (Objects.nonNull(session)) { diff --git a/shenyu-admin/src/test/java/org/apache/shenyu/admin/listener/websocket/WebsocketCollectorTest.java b/shenyu-admin/src/test/java/org/apache/shenyu/admin/listener/websocket/WebsocketCollectorTest.java index 86d5a3a870..ed758a5506 100644 --- a/shenyu-admin/src/test/java/org/apache/shenyu/admin/listener/websocket/WebsocketCollectorTest.java +++ b/shenyu-admin/src/test/java/org/apache/shenyu/admin/listener/websocket/WebsocketCollectorTest.java @@ -17,6 +17,9 @@ package org.apache.shenyu.admin.listener.websocket; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import com.google.gson.JsonObject; import jakarta.websocket.RemoteEndpoint; import jakarta.websocket.SendHandler; @@ -635,6 +638,48 @@ public final class WebsocketCollectorTest { websocketCollector.onClose(session); } + @Test + void testSendWithNamespaceIdLogsBoundedMetadataNotThePayload() { + final RemoteEndpoint.Async async = mockSuccessfulAsyncRemote(session); + websocketCollector.onOpen(session); + // a realistic envelope carrying a secret; the collector must log bounded metadata, never the payload + final String message = "{\"groupType\":\"AI_PROXY_API_KEY\",\"eventType\":\"UPDATE\",\"data\":[{\"apiKey\":\"api-secret\"}]}"; + // Attach to the root logger: the collector's events reach it through additivity whether the collector's + // static LOG is the real logger or the @BeforeAll spy, whose copied parent chain still points at the root. + final ch.qos.logback.classic.Logger root = + (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME); + final ListAppender<ILoggingEvent> appender = new ListAppender<>(); + appender.start(); + final Level original = root.getLevel(); + root.setLevel(Level.INFO); + root.addAppender(appender); + try { + WebsocketCollector.send(Constants.SYS_DEFAULT_NAMESPACE_ID, message, DataEventTypeEnum.CREATE); + } finally { + root.detachAppender(appender); + root.setLevel(original); + } + // delivery is preserved even though the diagnostic no longer serializes the payload + verify(async, times(1)).sendText(eq(message), any(SendHandler.class)); + // exactly one bounded INFO diagnostic is emitted, and no DEBUG redaction/serialization runs at INFO + final String collectorName = WebsocketCollector.class.getName(); + assertEquals(1L, appender.list.stream() + .filter(event -> collectorName.equals(event.getLoggerName())) + .count(), "expected exactly one bounded collector diagnostic"); + final String logged = appender.list.stream() + .filter(event -> collectorName.equals(event.getLoggerName())) + .map(ILoggingEvent::getFormattedMessage) + .findFirst() + .orElse(""); + assertTrue(logged.contains(Constants.SYS_DEFAULT_NAMESPACE_ID), logged); + assertTrue(logged.contains(DataEventTypeEnum.CREATE.name()), logged); + assertTrue(logged.contains(String.valueOf(message.length())), logged); + // neither the payload shape, nor the secret, nor the redaction marker may be serialized into the log + assertFalse(logged.contains("groupType"), logged); + assertFalse(logged.contains("api-secret"), logged); + assertFalse(logged.contains("******"), logged); + } + @Test void testSendBySessionFailure() throws Exception { final RemoteEndpoint.Async async = mock(RemoteEndpoint.Async.class);
