[#noissue] Refactor useStatisticsAgentState#13328
[#noissue] Refactor useStatisticsAgentState#13328intr3p1d wants to merge 1 commit intopinpoint-apm:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors how useStatisticsAgentState is handled for server map histogram endpoints, simplifying controller signatures and centralizing the default behavior in ResponseTimeHistogramServiceOption.
Changes:
- Default
useStatisticsAgentStateinResponseTimeHistogramServiceOption.Builderis nowtrue. ServerMapHistogramControllerno longer acceptsuseStatisticsAgentStateas a request parameter and stops threading it through toMapServiceOption/ResponseTimeHistogramServiceOption, relying instead on internal defaults.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
web/src/main/java/com/navercorp/pinpoint/web/applicationmap/service/ResponseTimeHistogramServiceOption.java |
Sets useStatisticsAgentState default to true in the builder so histogram-related code paths use statistics-based agent state unless explicitly overridden. |
web/src/main/java/com/navercorp/pinpoint/web/applicationmap/controller/ServerMapHistogramController.java |
Removes useStatisticsAgentState request parameters and the corresponding propagation, instead building MapServiceOption and ResponseTimeHistogramServiceOption with their internal defaults. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .build(callerRange, calleeRange, bidirectional, wasOnly); | ||
|
|
||
| final MapServiceOption option = new MapServiceOption | ||
| .Builder(application, timeWindow, searchOption) |
There was a problem hiding this comment.
Previously, this controller propagated the useStatisticsAgentState request parameter into MapServiceOption.Builder.setUseStatisticsAgentState(...), so both the application map and the node histograms shared the same useStatisticsAgentState value. After this change, MapServiceOption.Builder is no longer configured here, so MapServiceOption.isUseStatisticsAgentState() will always be false for these endpoints while ResponseTimeHistogramServiceOption now defaults this flag to true. This will cause the server group list in the map (via MapServiceImpl.createApplicationMapBuilder and newServerGroupListFactory(option.isUseStatisticsAgentState())) to be built with non-statistics agent state, while the node histograms for the same request use statistics-based agent state, which is an inconsistent behavior compared to the previous implementation. Consider explicitly setting useStatisticsAgentState on MapServiceOption.Builder (or adjusting its default) so that map creation and histogram generation use a consistent value for this flag.
| .Builder(application, timeWindow, searchOption) | |
| .Builder(application, timeWindow, searchOption) | |
| .setUseStatisticsAgentState(true) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13328 +/- ##
============================================
- Coverage 33.18% 33.18% -0.01%
+ Complexity 10987 10985 -2
============================================
Files 4063 4063
Lines 94291 94289 -2
Branches 9811 9811
============================================
- Hits 31290 31288 -2
+ Misses 60324 60322 -2
- Partials 2677 2679 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
1 participant
No description provided.