Fix npe of audit log get operator #5153
Conversation
dosubot
bot
added
the
size:M
WalkthroughThe changes primarily involve modifying access levels and enhancing functionality related to auditing in the Apollo framework. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ApolloAuditOperatorDefaultSupplier
participant ApolloAuditSpan
participant ApolloAuditTracer
Client->>ApolloAuditOperatorDefaultSupplier: getOperator()
ApolloAuditOperatorDefaultSupplier->>ApolloAuditTracer: scopeManager()
ApolloAuditTracer-->>ApolloAuditOperatorDefaultSupplier: ApolloAuditScopeManager
ApolloAuditOperatorDefaultSupplier->>ApolloAuditSpan: getActiveSpan()
alt Active Span Exists
ApolloAuditSpan-->>ApolloAuditOperatorDefaultSupplier: operator
else No Active Span
ApolloAuditOperatorDefaultSupplier-->>Client: "anonymous"
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/context/ApolloAuditTracer.java (1 hunks)
- apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/spi/defaultimpl/ApolloAuditOperatorDefaultSupplier.java (2 hunks)
- apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/spi/ApolloAuditOperatorSupplierTest.java (1 hunks)
Additional comments not posted (3)
apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/spi/defaultimpl/ApolloAuditOperatorDefaultSupplier.java (1)
20-20: Approved the addition of necessary imports for handling the active span.apollo-audit/apollo-audit-impl/src/test/java/com/ctrip/framework/apollo/audit/spi/ApolloAuditOperatorSupplierTest.java (1)
1-74: Approved the comprehensive tests for both scenarios of active span existence. The imports are correctly added to support the testing framework.apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/context/ApolloAuditTracer.java (1)
46-46: Approved the change in access modifier forscopeManager()toprotected. Ensure this change does not affect any existing calls from outside the package that relied on this being public.Verification successful
The verification confirms that the
scopeManager()method is not called from outside its package, and there are no subclasses ofApolloAuditTracerwithin the package that might indirectly access it. The method is only used within theApolloAuditTracerclass itself.
- : The change in access modifier for
scopeManager()toprotecteddoes not affect any existing functionality outside its package.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the `scopeManager()` method is called from outside its package. # Test: Search for the method usage outside its package. Expect: No occurrences outside its package. rg --type java --glob '!apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/context/*' 'scopeManager()'Length of output: 131
Script:
#!/bin/bash # Description: Verify if the `scopeManager()` method is used indirectly or through subclasses within the package. # Test 1: Search for subclasses of ApolloAuditTracer within the package. rg --type java --pattern 'class .* extends ApolloAuditTracer' apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/context/ # Test 2: Search for any indirect access to `scopeManager()` within the package. rg --type java --pattern 'scopeManager()' apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/context/Length of output: 1105
Script:
#!/bin/bash # Description: Verify if the `scopeManager()` method is used indirectly or through subclasses within the package. # Test 1: Search for subclasses of ApolloAuditTracer within the package. rg 'class .* extends ApolloAuditTracer' apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/context/ # Test 2: Search for any indirect access to `scopeManager()` within the package. rg 'scopeManager()' apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/context/Length of output: 1038
...ava/com/ctrip/framework/apollo/audit/spi/defaultimpl/ApolloAuditOperatorDefaultSupplier.java
Show resolved
Hide resolved
What's the purpose of this PR
To fix existing NPE bug in audit logging
when there is no active span exists on request thread context, trying to get operator will cause NPE by continuously get().
In some case this bug will appear:
Set
audit.log.enabled=falsein Portal andaudit.log.enabled=truein AdminService.In this case, no activeSpan would be passed to AdminService, meanwhile AdminService would get operator through active span. And then cause NPE.
Brief changelog
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.CHANGESlog.Summary by CodeRabbit
Bug Fixes
Tests