[BUG] AbstractLogCollector.desensitizeShenyuRequestLog NPE on null boxed Integer/Long (chunked byte responses)

[Aias00]

Description

new ThreadPoolExecutor(threadCount, MAX_ALLOW_THREADS, 60000L, TimeUnit.MICROSECONDS, …) — 60000L with TimeUnit.MICROSECONDS = 60 ms, not the intended 60 seconds. Burst-spawned non-core threads die after only 60 ms of idle.

Location

• shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-tencent-cls/src/main/java/org/apache/shenyu/plugin/tencent/cls/client/TencentClsLogCollectClient.java:163
• shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-aliyun-sls/src/main/java/org/apache/shenyu/plugin/aliyun/sls/client/AliyunSlsLogCollectClient.java:184

Impact

Callback-executor threads cycle rapidly on any pause in traffic; wasted thread creation/teardown.

Suggested fix

Use TimeUnit.MILLISECONDS (or SECONDS with 60L).

Related existing

None — distinct from #6794 (Logging pools AbortPolicy + unit bug) which covers the send pools in different clients.

[github-actions]

👋 Hello @yuluo-yx

A new issue has been opened by @Aias00:

• Title: [BUG] AbstractLogCollector.desensitizeShenyuRequestLog NPE on null boxed Integer/Long (chunked byte responses)
• Link: [BUG] AbstractLogCollector.desensitizeShenyuRequestLog NPE on null boxed Integer/Long (chunked byte responses) #6884

Please review when you have time. Thank you! 🙏

[nikhiln64]

I would like to take this one. A heads up on the batch first, the issue bodies filed on Aug 4 appear to be offset by one from their titles. The description matching this title actually sits in the body of #6883, this issue's body describes the keep alive unit bug titled in #6885, and the pattern continues through the batch. Might be worth realigning them so future readers land on the right analysis.

On the bug itself, confirmed in code. For byte type media appendResponse calls logResponse with a null writer, so when the response also has no Content-Length header responseContentLength is never set. desensitizeShenyuRequestLog then calls toString on it unconditionally and throws. status can stay null the same way because appendResponse only sets it when the status code is already committed, and upstreamResponseTime is nullable on the same class of paths. I have a fix with null guards on the three boxed fields plus a regression test, opening the PR now.