[#noissue] Refactor active agent validation to use agent ID and service type#13419
[#noissue] Refactor active agent validation to use agent ID and service type#13419emeroad merged 1 commit intopinpoint-apm:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request refactors the agent activity validation logic to use primitive parameters (agentId and agentServiceType) instead of Application objects. This simplifies method signatures across the batch and web modules and reduces coupling to the Application class.
Changes:
- Refactored
ActiveAgentValidatorinterface andDefaultActiveAgentValidatorimplementation to acceptagentId(String) andagentServiceType(int) parameters instead ofApplicationobjects - Updated
BatchAgentServiceinterface and implementation to match the new signature pattern - Modified
AlarmProcessor.fetchActiveAgents()to use the new API, replacing stream-based filtering with a for-loop
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/main/java/com/navercorp/pinpoint/web/service/component/ActiveAgentValidator.java | Updated interface methods to accept primitive parameters and removed Application import |
| web/src/main/java/com/navercorp/pinpoint/web/service/component/DefaultActiveAgentValidator.java | Implemented the refactored methods using agentId and agentServiceType parameters |
| batch/src/main/java/com/navercorp/pinpoint/batch/service/BatchAgentService.java | Updated service interface to match the new signature pattern |
| batch/src/main/java/com/navercorp/pinpoint/batch/service/BatchAgentServiceImpl.java | Implemented the new method signature delegating to ActiveAgentValidator |
| batch/src/main/java/com/navercorp/pinpoint/batch/alarm/AlarmProcessor.java | Updated to call the new API with agentId and service type code directly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| boolean isActiveAgent(String agentId, int agentServiceType, Range range); | ||
|
|
||
| boolean isActiveAgent(Application agent, String version, Range range); | ||
| boolean isActiveAgent(String agentId, int serviceType, String version, Range range); |
There was a problem hiding this comment.
Parameter name inconsistency: this method uses 'serviceType' while the overloaded method on line 8 uses 'agentServiceType' for the same semantic parameter. For consistency, this should be renamed to 'agentServiceType' to match the other overload and the implementation in DefaultActiveAgentValidator.
| boolean isActiveAgent(String agentId, int serviceType, String version, Range range); | |
| boolean isActiveAgent(String agentId, int agentServiceType, String version, Range range); |
| public boolean isActiveAgent(String agentId, int agentServiceType, Range range) { | ||
| return isActiveAgent(agentId, agentServiceType, null, range); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isActiveAgent(Application agent, String version, Range range) { | ||
| Objects.requireNonNull(agent, "agent"); | ||
| String agentId = agent.getName(); | ||
| if (!agentCompatibility.isLegacyAgent(agent.getServiceTypeCode(), version)) { | ||
| public boolean isActiveAgent(String agentId, int agentServiceType, String version, Range range) { |
There was a problem hiding this comment.
The method signature change removes the overload that accepts an Application object. However, the test file DefaultActiveAgentValidatorTest.java (lines 35, 46, 54, 67) still calls isActiveAgent with an Application object, which will cause compilation failures. The tests need to be updated to use the new signature that accepts agentId (String) and agentServiceType (int) instead.
emeroad
force-pushed
the
#noissue_valiation
branch
from
9a29adf to
561ff2c
Compare
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #13419 +/- ##
============================================
- Coverage 33.10% 33.10% -0.01%
Complexity 10974 10974
============================================
Files 4066 4066
Lines 94350 94350
Branches 9815 9817 +2
============================================
- Hits 31235 31231 -4
- Misses 60432 60436 +4
Partials 2683 2683 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
…ce type
This pull request refactors how agent activity is checked by removing the dependency on the
Applicationobject and instead using primitive parameters (agentIdandagentServiceType). This simplifies method signatures and improves clarity in the agent activity validation logic across the batch and web modules.Refactoring agent activity validation:
BatchAgentService.isActiveand its implementations to acceptagentIdandagentServiceTypeas parameters instead of anApplicationobject, updating all usages accordingly (BatchAgentService.java,BatchAgentServiceImpl.java,AlarmProcessor.java). [1] [2] [3]ActiveAgentValidatorinterface and its implementation to replace methods usingApplicationwith methods usingagentIdandagentServiceType, and adjusted the validation logic to match (ActiveAgentValidator.java,DefaultActiveAgentValidator.java). [1] [2]Code cleanup:
Applicationfrom affected files, reflecting the updated method signatures. [1] [2] [3]