[#noissue] Refactor parsing logic to use `KeyValueTokenizer` #13315

emeroad · 2026-01-20T11:13:20Z

This pull request introduces a new utility class, KeyValueTokenizer, to standardize and simplify the parsing of key-value pairs across the codebase. The change replaces various manual string splitting logic with this centralized utility, improving code readability and reducing duplication. The update also includes comprehensive unit tests for the new utility and refactors several modules to use it.

Key changes include:

1. Introduction of KeyValueTokenizer utility:

Added KeyValueTokenizer in commons/src/main/java/com/navercorp/pinpoint/common/util/KeyValueTokenizer.java, providing methods for safe, null-aware key-value parsing with customizable factories for different value types.

2. Refactoring to use KeyValueTokenizer:

Replaced manual string splitting with KeyValueTokenizer in:
- Dameng JDBC URL parsing and query parameter parsing (DamengJdbcUrlParser.java). [1] [2] [3]
- gRPC Channelz socket lookup logic (ChannelzSocketLookup.java). [1] [2]
- Exception trace query parameter filter parsing (ExceptionTraceQueryParameter.java). [1] [2]
- Metric tag parsing utilities (TagUtils.java). [1] [2] [3]
- Redis channel key parsing (RedisKVPubChannelProvider.java, RedisKVSubChannelProvider.java). [1] [2] [3] [4]

3. Testing improvements:

Added a dedicated test class, KeyValueTokenizerTest, with thorough unit tests covering various edge cases for the new utility.
Added a new test for empty value parsing in metric tag utilities (TagUtilsTest.java).

4. Minor related improvements:

Simplified and clarified tag string handling in TagUtils (e.g., improved JSON tag string cleaning).
Minor fix to ensure split methods in ServerMapHistogramController only split into two parts for node and link key parsing. [1] [2]

These changes collectively improve code maintainability, reduce the risk of subtle bugs in key-value parsing, and make the parsing logic more consistent across the project.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T12:32:30Z

...dameng-jdbc/src/main/java/com/navercorp/pinpoint/plugin/jdbc/dameng/DamengJdbcUrlParser.java

-                map.put(kv[0], StringUtils.EMPTY_STRING);
-            }
+            KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize(v, "=");
+            map.put(keyValue.getKey(), keyValue.getValue());


Potential null pointer exception if KeyValueTokenizer.tokenize() returns null when the delimiter is not found. The code should check if keyValue is null before calling getKey() and getValue(), or handle the case where the query parameter doesn't contain '='.

Suggested change

map.put(keyValue.getKey(), keyValue.getValue());

if (keyValue == null) {

// Handle parameters without '=' by using the whole segment as key and an empty value

map.put(v, "");

} else {

map.put(keyValue.getKey(), keyValue.getValue());

}

Copilot · 2026-01-20T12:32:31Z

redis/src/main/java/com/navercorp/pinpoint/channel/redis/kv/RedisKVSubChannelProvider.java

-        if (words.length != 2) {
+        KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize(key, ":");
+        if (keyValue == null) {
            throw new IllegalArgumentException("the key must contain period");


The error message says 'the key must contain period' but the delimiter used is ':', not a period. The message should be corrected to match the actual delimiter.

Suggested change

throw new IllegalArgumentException("the key must contain period");

throw new IllegalArgumentException("the key must contain ':'");

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-21T02:09:35Z

commons-server/src/test/java/com/navercorp/pinpoint/common/util/KeyValueTokenizerTest.java

+class KeyValueTokenizerTest {
+
+    @Test
+    void tokenize() {
+        KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize("key=value", "=");
+        assertEquals("key", keyValue.getKey());
+        assertEquals("value", keyValue.getValue());
+    }
+
+    @Test
+    void tokenize2() {
+        KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize("key==value", "==");
+        assertEquals("key", keyValue.getKey());
+        assertEquals("value", keyValue.getValue());
+    }
+
+    @Test
+    void tokenize_emptyValue() {
+        KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize("key=", "=");
+        assertEquals("key", keyValue.getKey());
+        assertEquals("", keyValue.getValue());
+    }
+
+    @Test
+    void tokenize_emptyKeyValue() {
+        KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize("=", "=");
+        assertEquals("", keyValue.getKey());
+        assertEquals("", keyValue.getValue());
+    }
+
+    @Test
+    void tokenize_empty() {
+        KeyValueTokenizer.KeyValue keyValue = KeyValueTokenizer.tokenize("", "=");
+        Assertions.assertNull(keyValue);
+    }
+}


The KeyValueTokenizerTest is missing a test case for when a null text parameter is passed to the tokenize method. According to line 32 of KeyValueTokenizer.java, the method will throw a NullPointerException if text is null. A test should be added to document this expected behavior, either verifying that a NullPointerException is thrown or that the method handles null gracefully.

Copilot · 2026-01-21T02:09:36Z

...ule/metric-commons/src/test/java/com/navercorp/pinpoint/metric/common/util/TagUtilsTest.java

+    @Test
+    public void parseTagTest_emptyValue() {
+        Tag tag = TagUtils.parseTag("A:");
+
+        Assertions.assertEquals("A", tag.getName());
+        Assertions.assertEquals("", tag.getValue());
+    }


The test suite is missing a test case for when parseTag is called with a string that doesn't contain a colon delimiter. This is particularly important since the refactoring changed the behavior from the old implementation (which used StringUtils.split and handled missing delimiters by treating the entire string as the key with an empty value) to KeyValueTokenizer.tokenize (which returns null when the delimiter is not found). A test should verify the expected behavior in this case.

Copilot · 2026-01-21T02:09:37Z

commons/src/main/java/com/navercorp/pinpoint/common/util/KeyValueTokenizer.java

+     * @param text the text to tokenize
+     * @param delimiter the delimiter separating key and value
+     * @param factory the factory used to create the token
+     * @param <T> the type of token to return
+     * @return the parsed token, or {@code null} if the delimiter is not found in the text


The javadoc comment on line 29 states that the method returns "null if the delimiter is not found in the text", but this important behavior is not consistently documented or handled by all callers. Consider adding an @throws NullPointerException annotation for the text parameter, and potentially adding a note in the javadoc that callers should check for null return values.

Suggested change

* @param text the text to tokenize

* @param delimiter the delimiter separating key and value

* @param factory the factory used to create the token

* @param <T> the type of token to return

* @return the parsed token, or {@code null} if the delimiter is not found in the text

* @param text the text to tokenize; must not be {@code null}

* @param delimiter the delimiter separating key and value

* @param factory the factory used to create the token

* @param <T> the type of token to return

* @return the parsed token, or {@code null} if the delimiter is not found in the text; callers should

* check for a {@code null} return value

* @throws NullPointerException if {@code text} is {@code null}

emeroad added this to the 3.1.0 milestone Jan 20, 2026

emeroad requested a review from Copilot January 20, 2026 11:13

Copilot started reviewing on behalf of emeroad January 20, 2026 11:13 View session

This comment was marked as outdated.

Sign in to view

emeroad requested a review from Copilot January 20, 2026 12:22

Copilot started reviewing on behalf of emeroad January 20, 2026 12:22 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

emeroad force-pushed the #noissue_cleanup6 branch from 44fcb19 to 7cfb618 Compare January 21, 2026 01:58

[#noissue] Refactor parsing logic to use KeyValueTokenizer

3bf69d7

emeroad changed the title ~~[#noissue] Refactor parsing logic to use KeyValueTokenizer.~~ [#noissue] Refactor parsing logic to use KeyValueTokenizer Jan 21, 2026

emeroad force-pushed the #noissue_cleanup6 branch from 7cfb618 to 3bf69d7 Compare January 21, 2026 01:58

emeroad requested a review from Copilot January 21, 2026 01:59

Copilot started reviewing on behalf of emeroad January 21, 2026 01:59 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[#noissue] Refactor parsing logic to use `KeyValueTokenizer` #13315

[#noissue] Refactor parsing logic to use `KeyValueTokenizer` #13315

emeroad commented Jan 20, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	throw new IllegalArgumentException("the key must contain period");
	throw new IllegalArgumentException("the key must contain ':'");

[#noissue] Refactor parsing logic to use KeyValueTokenizer #13315

Are you sure you want to change the base?

[#noissue] Refactor parsing logic to use KeyValueTokenizer #13315

Conversation

emeroad commented Jan 20, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

[#noissue] Refactor parsing logic to use `KeyValueTokenizer` #13315

[#noissue] Refactor parsing logic to use `KeyValueTokenizer` #13315