[#noissue] Refactor application name retrieval to use getApplicationName()#13395
Open
emeroad wants to merge 1 commit intopinpoint-apm:masterfrom
Open
[#noissue] Refactor application name retrieval to use getApplicationName()#13395emeroad wants to merge 1 commit intopinpoint-apm:masterfrom
emeroad wants to merge 1 commit intopinpoint-apm:masterfrom
Conversation
|
There was a problem hiding this comment.
Pull request overview
This pull request performs a comprehensive refactoring to standardize application name retrieval across the Pinpoint codebase by replacing all calls to Application.getName() with Application.getApplicationName(). The refactoring also introduces a new Service class and updates the Application class to include a service field, setting up infrastructure for future multi-tenancy support while maintaining backward compatibility.
Changes:
- Deprecated
getName()method onApplicationclass and replaced all usages withgetApplicationName()across the entire codebase - Added new
Serviceclass to support future service-level multi-tenancy - Updated
Applicationclass to include aServicefield (defaulting toService.DEFAULTfor backward compatibility) - Applied changes consistently across web modules, batch jobs, alarm processors, data collectors, DAOs, mappers, serializers, and all test files
Reviewed changes
Copilot reviewed 65 out of 65 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/src/main/java/com/navercorp/pinpoint/web/vo/Service.java | New class introducing Service concept with uid and name for multi-tenancy support |
| web/src/main/java/com/navercorp/pinpoint/web/vo/Application.java | Added Service field, renamed internal field to applicationName, deprecated getName(), added getApplicationName() |
| web/src/main/java/com/navercorp/pinpoint/web/component/DefaultApplicationFactory.java | Updated to construct Application with Service.DEFAULT |
| web/src/main/java/com/navercorp/pinpoint/web/view/ApplicationSerializer.java | Updated JSON serialization to use getApplicationName() |
| web/src/main/java/com/navercorp/pinpoint/web/view/ApplicationScatterScanResultSerializer.java | Updated JSON serialization to use getApplicationName() and reordered imports |
| web/src/main/java/com/navercorp/pinpoint/web/view/id/AgentNameView.java | Updated to use getApplicationName() |
| web/src/main/java/com/navercorp/pinpoint/web/view/histogram/*.java | Updated histogram views to use getApplicationName() |
| web/src/main/java/com/navercorp/pinpoint/web/service/*.java | Updated service implementations to use getApplicationName() for operations |
| web/src/main/java/com/navercorp/pinpoint/web/applicationmap/**/*.java | Updated all application map related classes (nodes, links, histograms, DAOs, mappers) to use getApplicationName() |
| web/src/main/java/com/navercorp/pinpoint/web/agentlist/service/AgentsServiceImpl.java | Updated to use getApplicationName() with import reordering |
| web/src/main/java/com/navercorp/pinpoint/web/authorization/controller/AdminController.java | Updated admin operations to use getApplicationName() |
| web/src/main/java/com/navercorp/pinpoint/web/uid/service/ApplicationIndexV2CopyServiceImpl.java | Updated V2 index copy operations to use getApplicationName() |
| web/src/main/java/com/navercorp/pinpoint/web/vo/ResponseHistograms.java | Updated response histogram builder to use getApplicationName() |
| batch/src/main/java/com/navercorp/pinpoint/batch/alarm/*.java | Updated alarm reader and processor to use getApplicationName() |
| batch/src/main/java/com/navercorp/pinpoint/batch/alarm/collector/**/*.java | Updated all data collectors to use getApplicationName() for DAO queries and logging |
| batch/src/main/java/com/navercorp/pinpoint/batch/job/*.java | Updated batch jobs for application and agent management to use getApplicationName() |
| batch/src/main/java/com/navercorp/pinpoint/batch/service/BatchApplicationIndexServiceImpl.java | Updated batch application index service to use getApplicationName() |
| web/src/test/**/*.java | Updated all test files to use getApplicationName() consistently with production code |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,48 @@ | |||
| package com.navercorp.pinpoint.web.vo; | |||
There was a problem hiding this comment.
The Service class is missing the standard Apache License 2.0 copyright header that is present in other files in this codebase. Add the copyright header to maintain consistency with the project's licensing requirements.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #13395 +/- ##
============================================
- Coverage 33.11% 33.10% -0.01%
- Complexity 10967 10968 +1
============================================
Files 4066 4067 +1
Lines 94295 94314 +19
Branches 9811 9812 +1
============================================
+ Hits 31225 31227 +2
- Misses 60389 60404 +15
- Partials 2681 2683 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
1 participant
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.
…ame()
This pull request standardizes the way application names are accessed throughout the codebase by replacing calls to
getName()withgetApplicationName()on theApplicationobject. This change improves consistency and helps prevent potential bugs related to ambiguous method naming. The update is applied across multiple modules, including alarm processing, data collection, application management, and associated tests.Core Refactoring:
application.getName()withapplication.getApplicationName()in alarm processing logic, including rule selection and agent ID fetching inAlarmProcessor.javaandAlarmReader.java. [1] [2] [3]MapOutLinkDataCollector,DataSourceDataCollector,FileDescriptorDataCollector,HeapDataCollector,JvmCpuDataCollector,SystemCpuDataCollector) to usegetApplicationName()for DAO queries and logging. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Application and Agent Management:
AgentRemover,ApplicationCleaningProcessor,ApplicationRemover,BatchApplicationIndexServiceImpl) to usegetApplicationName()for agent and application operations. [1] [2] [3] [4]Web and API Layer Updates:
AgentsServiceImpl,AgentInfoServerGroupListDataSource,ApplicationResponse,MapScanKeyFactoryV2) to consistently usegetApplicationName()when interacting with application-related data. [1] [2] [3] [4] [5]Test Code Consistency:
getApplicationName()for mocking and verification, ensuring test consistency with production code. [1] [2]These changes collectively enhance code clarity and reduce the risk of future errors related to application name retrieval.