Chargebee telemetry handling#109
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
WalkthroughAdds RFC 9651 ChangesResponse telemetry header
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Transport
participant TelemetryExecutor
participant TelemetrySupport
participant ChargebeeTelemetryHeaderParser
participant Span
Transport->>TelemetryExecutor: Return response with telemetry header
TelemetryExecutor->>TelemetrySupport: Pass response headers
TelemetrySupport->>ChargebeeTelemetryHeaderParser: Parse telemetry header
ChargebeeTelemetryHeaderParser-->>TelemetrySupport: Return typed attributes
TelemetrySupport->>Span: Add raw and parsed attributes
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 863-867: Update the List branch in the README example to
create/use an AttributeKey via AttributeKey.stringArrayKey for the attribute
name before calling span.setAttribute, while preserving the existing String list
conversion and suppression behavior.
In
`@src/main/java/com/chargebee/v4/telemetry/ChargebeeTelemetryHeaderParser.java`:
- Around line 178-224: Update splitOnDelimiter, indexOfParameterSeparator, and
indexOfEquals to recognize escaped quotes and avoid toggling inQuotes when a
quote is preceded by an escape character. Ensure delimiters and equals signs
inside quoted values such as desc="a\"b;c" remain part of the value, while
unescaped quotes continue to change quote state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 24b5d2f6-4f0b-4078-bbfb-3695f34701d6
📒 Files selected for processing (8)
README.mdsrc/main/java/com/chargebee/v4/telemetry/ChargebeeTelemetryHeaderParser.javasrc/main/java/com/chargebee/v4/telemetry/TelemetryAttributeKeys.javasrc/main/java/com/chargebee/v4/telemetry/TelemetryExecutor.javasrc/main/java/com/chargebee/v4/telemetry/TelemetrySupport.javasrc/test/java/com/chargebee/v4/telemetry/ChargebeeTelemetryHeaderParserTest.javasrc/test/java/com/chargebee/v4/telemetry/TelemetryExecutorTest.javasrc/test/java/com/chargebee/v4/telemetry/TelemetrySupportTest.java
| } else if (v instanceof List) { | ||
| @SuppressWarnings("unchecked") | ||
| List<String> values = (List<String>) v; | ||
| span.setAttribute(k, values); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
OpenTelemetry Java Span setAttribute overloads AttributeKey string array List<String> compile
💡 Result:
In OpenTelemetry Java, the Span interface uses a generic setAttribute method when working with AttributeKey instances [1][2]. To set a string array (or any list-based attribute), you must use an AttributeKey defined for a List type, such as one created via AttributeKey.stringArrayKey(String) [3][4]. The Span.setAttribute method signature is defined as: Span setAttribute(AttributeKey key, T value); When using this with a string array, the value must be a List, not a raw array [3][5]. Example compilation snippet: import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.Span; import java.util.List; // Define the key for a List of Strings AttributeKey<List> MY_ARRAY_KEY = AttributeKey.stringArrayKey("my.string.array"); // Use the key with the span Span span = Span.current; span.setAttribute(MY_ARRAY_KEY, List.of("value1", "value2")); While there are convenience methods on AttributesBuilder that support varargs (e.g., put(String key, String... values)) [6], the primary Span.setAttribute method using AttributeKey requires the value to match the generic type T exactly—in this case, a List [3][1]. If you have a raw array, you must convert it to a List (e.g., via Arrays.asList or List.of) to satisfy the compiler [7].
Citations:
- 1: https://www.javadoc.io/static/io.opentelemetry/opentelemetry-api/1.5.0/io/opentelemetry/api/trace/SpanBuilder.html
- 2: https://www.javadoc.io/static/io.opentelemetry/opentelemetry-api/1.9.0/io/opentelemetry/api/trace/Span.html
- 3: https://javadoc.io/static/io.opentelemetry/opentelemetry-api/1.51.0/io/opentelemetry/api/common/AttributeKey.html
- 4: https://javadoc.io/static/io.opentelemetry/opentelemetry-api/1.1.0/io/opentelemetry/api/common/AttributeKey.html
- 5: https://github.com/open-telemetry/opentelemetry-java/blob/main/api/all/src/main/java/io/opentelemetry/api/common/ArrayBackedAttributes.java
- 6: https://javadoc.io/static/io.opentelemetry/opentelemetry-api/1.1.0/io/opentelemetry/api/common/AttributesBuilder.html
- 7: Vararg versions of AttributeBuilder.put(AttributeKey<List<?>>) open-telemetry/opentelemetry-java#4172
🏁 Script executed:
python3 - <<'PY'
import sys, textwrap, json, os, subprocess, pathlib
print("Need web search results.")
PYRepository: chargebee/chargebee-java
Length of output: 187
Use AttributeKey.stringArrayKey for list attributes
Span.setAttribute only accepts AttributeKey<T> for non-scalar values, so span.setAttribute(k, values) with a String key and List<String> value has no matching overload. Use a string-array key here:
Fix
- span.setAttribute(k, values);
+ span.setAttribute(AttributeKey.stringArrayKey(k), values);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if (v instanceof List) { | |
| @SuppressWarnings("unchecked") | |
| List<String> values = (List<String>) v; | |
| span.setAttribute(k, values); | |
| } | |
| } else if (v instanceof List) { | |
| `@SuppressWarnings`("unchecked") | |
| List<String> values = (List<String>) v; | |
| span.setAttribute(AttributeKey.stringArrayKey(k), values); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 863 - 867, Update the List branch in the README
example to create/use an AttributeKey via AttributeKey.stringArrayKey for the
attribute name before calling span.setAttribute, while preserving the existing
String list conversion and suppression behavior.
| private static List<String> splitOnDelimiter(String input, char delimiter) { | ||
| List<String> parts = new ArrayList<>(); | ||
| StringBuilder current = new StringBuilder(); | ||
| boolean inQuotes = false; | ||
|
|
||
| for (int i = 0; i < input.length(); i++) { | ||
| char currentChar = input.charAt(i); | ||
| if (currentChar == '"') { | ||
| inQuotes = !inQuotes; | ||
| current.append(currentChar); | ||
| } else if (currentChar == delimiter && !inQuotes) { | ||
| addIfNotBlank(parts, current); | ||
| current = new StringBuilder(); | ||
| } else { | ||
| current.append(currentChar); | ||
| } | ||
| } | ||
|
|
||
| addIfNotBlank(parts, current); | ||
| return parts; | ||
| } | ||
|
|
||
| private static int indexOfParameterSeparator(String item) { | ||
| boolean inQuotes = false; | ||
| for (int i = 0; i < item.length(); i++) { | ||
| char current = item.charAt(i); | ||
| if (current == '"') { | ||
| inQuotes = !inQuotes; | ||
| } else if (current == ';' && !inQuotes) { | ||
| return i; | ||
| } | ||
| } | ||
| return -1; | ||
| } | ||
|
|
||
| private static int indexOfEquals(String parameter) { | ||
| boolean inQuotes = false; | ||
| for (int i = 0; i < parameter.length(); i++) { | ||
| char current = parameter.charAt(i); | ||
| if (current == '"') { | ||
| inQuotes = !inQuotes; | ||
| } else if (current == '=' && !inQuotes) { | ||
| return i; | ||
| } | ||
| } | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='src/main/java/com/chargebee/v4/telemetry/ChargebeeTelemetryHeaderParser.java'
wc -l "$file"
echo '--- outline ---'
ast-grep outline "$file" --view expanded || true
echo '--- relevant lines ---'
sed -n '1,260p' "$file" | cat -nRepository: chargebee/chargebee-java
Length of output: 10501
Skip escaped quotes while scanning for delimiters. splitOnDelimiter, indexOfParameterSeparator, and indexOfEquals toggle quote state on every "; a value like desc="a\"b;c" can be split at the ;, and parseToSpanAttributes will drop the whole header on the resulting parse error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/chargebee/v4/telemetry/ChargebeeTelemetryHeaderParser.java`
around lines 178 - 224, Update splitOnDelimiter, indexOfParameterSeparator, and
indexOfEquals to recognize escaped quotes and avoid toggling inQuotes when a
quote is preceded by an escape character. Ensure delimiters and equals signs
inside quoted values such as desc="a\"b;c" remain part of the value, while
unescaped quotes continue to change quote state.
TBA
Adds Chargebee telemetry response-header handling for synchronous and asynchronous requests, including RFC 9651 parsing into typed OpenTelemetry span attributes. Captures headers on successful and failed responses, adds telemetry constants and documentation, and introduces comprehensive parser and integration tests covering valid, absent, and malformed headers.