Repository navigation
[type:refactor] refactor shenyu selector and rule cache - #4578
Merged
Merged
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4578 +/- ##
============================================
- Coverage 67.44% 67.42% -0.02%
- Complexity 7959 8000 +41
============================================
Files 1095 1096 +1
Lines 31296 31529 +233
Branches 2813 2859 +46
============================================
+ Hits 21108 21260 +152
- Misses 8531 8596 +65
- Partials 1657 1673 +16
... and 14 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
yu199195
approved these changes
May 16, 2023
Aias00
pushed a commit
that referenced
this pull request
Oct 17, 2026
…7643) (#7663) * fix(plugin-base): route match-success logging through the logged flag Two cases where the code contradicted itself. #7643's scope asks for routine match-success diagnostics to sit behind DEBUG or the explicit matching-log setting, without duplicating messages. printLog already sits behind the setting: it checks selectorData.getLogged() and ruleData.getLoged() and carries the selector or rule name. defaultMatchSelector and defaultMatchRule logged the same event a second time with no switch at all, so a user who had not enabled logging still received one INFO line per matched request while the cache-hit path stayed quiet. Both lines arrived from the same refactor (#4578) that introduced printLog, and both are unreachable-as-documented once printLog runs, so they are removed rather than demoted -- demoting them to DEBUG would have left two DEBUG lines describing one match, which is the duplication the scope warns against. The surviving INFO line is the one that names the selector or rule. ShenyuWebsocketClient#handleResult logged the entire raw frame at INFO with no switch and no level check, emitted before the GsonUtils.fromJson call, so a malformed frame was written out in full before the catch discarded it. The payload moves to DEBUG unchanged and INFO carries only groupType, eventType, namespaceId and payload size, logged after parsing so the group type is available. Parse failures keep their WARN. The two removed lines cost 51 and 44 bytes of message text per match (95 bytes before logback's line prefix) for a default plugin name; a deployment matching on every request wrote that much per request to INFO regardless of the logged setting. That figure is derived from the message constants, not measured, and I have no controlled environment here to report a before/after rate, an allocation rate or a p99 -- the repo has no JMH infrastructure and I don't have a bare-metal Linux box, so I am not going to present numbers I can't defend. Tests cover both directions of the setting and the routing result: two cases assert no selector or rule line reaches INFO when the flag is false, two assert exactly one line does reach INFO when it is true and that it is the name-carrying one, and one asserts doExecute still receives the same selector and rule. Removing the two getLogged and getLoged guards makes the disabled cases fail, so they do pin the behaviour. The exactly-once assertions also pin the de-duplication. logback-classic is added with test scope, following shenyu-plugin-wasm-api and shenyu-sync-data-http, since this module had only slf4j-api on the test classpath. No new configuration flag and no change to matching or routing logic. Fixes #7643 * fix(sync): report websocket payload size in bytes Review follow-ups on the log-noise change: - ShenyuWebsocketClient: count the payload as UTF-8 bytes instead of UTF-16 chars, so the INFO line's "bytes" unit stays accurate for non-ASCII configuration content. - LogCapture: drop the unused originalLevel field and its no-op restore (the logger level is never modified during capture), and add the missing trailing newline. AbstractShenyuPluginTest 13/13, ShenyuWebsocketClientTest 34/34.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make sure that:
./mvnw clean install -Dmaven.javadoc.skip=true.