[#12502] Add alias support for agent profiles#12505
[#12502] Add alias support for agent profiles#12505donghun-cho merged 1 commit intopinpoint-apm:masterfrom
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12505 +/- ##
============================================
- Coverage 33.58% 33.56% -0.03%
- Complexity 10746 10748 +2
============================================
Files 3942 3943 +1
Lines 91887 91945 +58
Branches 9558 9563 +5
============================================
Hits 30859 30859
- Misses 58387 58446 +59
+ Partials 2641 2640 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds alias support for agent profiles, allowing users to specify profile aliases that resolve to supported profiles.
- Introduces a new constant (PROFILE_ALIAS_KEY_PREFIX) to identify alias properties
- Implements alias resolution logic in ProfilePropertyLoader.java via the resolveAlias and splitAndTrim methods
- Updates the configuration file documentation to include alias usage instructions
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| agent-module/bootstraps/bootstrap/src/main/java/com/navercorp/pinpoint/bootstrap/util/ProfileConstants.java | Added constant for profile alias key prefix |
| agent-module/bootstraps/bootstrap/src/main/java/com/navercorp/pinpoint/bootstrap/config/ProfilePropertyLoader.java | Added alias resolution logic and helper method for splitting aliases |
| agent-module/agent/src/main/resources/pinpoint-root.config | Updated configuration documentation for profile aliases |
Comments suppressed due to low confidence (1)
agent-module/bootstraps/bootstrap/src/main/java/com/navercorp/pinpoint/bootstrap/config/ProfilePropertyLoader.java:179
- [nitpick] Consider renaming 'splitAndTrim' to a more descriptive name such as 'parseCsvAliases' to clarify that the method processes comma-separated alias strings.
private List<String> splitAndTrim(String str) {
| String aliasesStr = javaSystemProperty.getProperty(ProfileConstants.PROFILE_ALIAS_KEY_PREFIX + supportedProfileName); | ||
| if (aliasesStr == null) { | ||
| aliasesStr = defaultProperties.getProperty(ProfileConstants.PROFILE_ALIAS_KEY_PREFIX + supportedProfileName); | ||
| if (aliasesStr == null) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Consider extracting the alias lookup logic into a separate helper function to simplify the resolveAlias method and reduce nested conditions.
| String aliasesStr = javaSystemProperty.getProperty(ProfileConstants.PROFILE_ALIAS_KEY_PREFIX + supportedProfileName); | |
| if (aliasesStr == null) { | |
| aliasesStr = defaultProperties.getProperty(ProfileConstants.PROFILE_ALIAS_KEY_PREFIX + supportedProfileName); | |
| if (aliasesStr == null) { | |
| continue; | |
| } | |
| String aliasesStr = getAliasString(defaultProperties, supportedProfileName); | |
| if (aliasesStr == null) { | |
| continue; |
There was a problem hiding this comment.
Pull Request Overview
Adds support for resolving profile aliases in the agent bootstrap configuration by introducing a new prefix constant, helper methods for property lookup and alias checking, and default alias entries in the root config.
- Introduce
PROFILE_ALIAS_KEY_PREFIXfor alias lookup - Implement
getProfileProperties(...)andcontainsAlias(...)helper methods - Integrate alias resolution in
getActiveProfileand update default config with alias entries
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| bootstrap/src/main/java/com/navercorp/pinpoint/bootstrap/util/ProfileConstants.java | Added PROFILE_ALIAS_KEY_PREFIX constant for alias support |
| bootstrap/src/main/java/com/navercorp/pinpoint/bootstrap/config/ProfilePropertyLoader.java | Added getProfileProperties, containsAlias, and alias-resolution logic in getActiveProfile |
| agent/src/main/resources/pinpoint-root.config | Documented alias config and added empty alias entries for release and local |
Comments suppressed due to low confidence (2)
agent-module/bootstraps/bootstrap/src/main/java/com/navercorp/pinpoint/bootstrap/config/ProfilePropertyLoader.java:134
- Alias resolution logic has been added but no corresponding unit tests. Please add tests for single and multiple aliases, case-insensitive matching, and fallback behavior in
getActiveProfile.
// handle alias
agent-module/bootstraps/bootstrap/src/main/java/com/navercorp/pinpoint/bootstrap/config/ProfilePropertyLoader.java:143
- [nitpick] The error message could be more actionable by including the list of supported profiles (and optionally aliases) to aid debugging.
throw new IllegalStateException("unsupported profile:" + profile);
| if (profile == null) { | ||
| profile = defaultProperties.getProperty(ProfileConstants.ACTIVE_PROFILE_KEY); | ||
| } | ||
| String profile = getProfileProperties(defaultProperties, ProfileConstants.ACTIVE_PROFILE_KEY); |
There was a problem hiding this comment.
An empty system or default property value will result in profile being an empty string (not null), which bypasses the null check and leads to an unhelpful exception. Consider using StringUtils.isEmpty(profile) to treat both null and empty as unset.
No description provided.