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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds opt-in response telemetry capture. The client sends the telemetry preference, telemetry execution preserves response headers, and ChangesResponse telemetry header
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ChargebeeClient
participant TelemetryExecutor
participant Transport
participant TelemetrySupport
participant ChargebeeTelemetryHeaderParser
participant OpenTelemetrySpan
ChargebeeClient->>TelemetryExecutor: telemetry preference
TelemetryExecutor->>Transport: request with optional Prefer header
Transport-->>TelemetryExecutor: response with X-Chargebee-Telemetry
TelemetryExecutor->>TelemetrySupport: response headers
TelemetrySupport->>ChargebeeTelemetryHeaderParser: telemetry header value
ChargebeeTelemetryHeaderParser-->>TelemetrySupport: typed attributes
TelemetrySupport->>OpenTelemetrySpan: raw and parsed attributes
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/chargebee/v4/telemetry/ChargebeeTelemetryHeaderParser.java (2)
150-167: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject invalid SF-string escapes and quotes.
parseStringValueaccepts\qand unescaped inner quotes. RFC 9651 permits only\"and\\. ThrowIllegalArgumentExceptionfor all other escapes and for"before the closing delimiter.🤖 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 150 - 167, Update parseStringValue to accept only escaped quote and backslash sequences, throwing IllegalArgumentException for any other escape character. While scanning the string, also reject unescaped inner double quotes before the final delimiter, while preserving valid decoded content and the existing unterminated-escape validation.
21-22: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnforce RFC 9651 numeric bounds before conversion.
The patterns accept invalid values such as
ratio=1.2345and a 16-digit integer. The@branch also bypasses these checks. Validate integer, decimal, and date grammar before numeric conversion so invalid telemetry uses the documented raw-only fallback.🤖 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 21 - 22, Update ChargebeeTelemetryHeaderParser’s numeric parsing to enforce RFC 9651 bounds and grammar before conversion: restrict integers and decimals to their documented digit and precision limits, validate date syntax similarly, and apply the same validation in the @ branch. Route any invalid value to the existing raw-only fallback rather than converting it.
🤖 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 `@src/test/java/com/chargebee/v4/telemetry/TelemetrySupportTest.java`:
- Around line 124-131: The telemetry Prefer helper is missing and must merge,
rather than suppress, existing preferences. Add the CHARGEBEE_TELEMETRY_PREFER_*
constants and implement TelemetrySupport.applyResponseTelemetryPreferHeader so
it appends chargebee-telemetry=include to any existing Prefer value while
preserving the existing value; update TelemetrySupportTest.java lines 124-131
and TelemetryExecutorTest.java lines 494-506 to assert both preferences.
---
Outside diff comments:
In
`@src/main/java/com/chargebee/v4/telemetry/ChargebeeTelemetryHeaderParser.java`:
- Around line 150-167: Update parseStringValue to accept only escaped quote and
backslash sequences, throwing IllegalArgumentException for any other escape
character. While scanning the string, also reject unescaped inner double quotes
before the final delimiter, while preserving valid decoded content and the
existing unterminated-escape validation.
- Around line 21-22: Update ChargebeeTelemetryHeaderParser’s numeric parsing to
enforce RFC 9651 bounds and grammar before conversion: restrict integers and
decimals to their documented digit and precision limits, validate date syntax
similarly, and apply the same validation in the @ branch. Route any invalid
value to the existing raw-only fallback rather than converting it.
🪄 Autofix
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: 7370c852-9502-48eb-a217-222e90180b5a
📒 Files selected for processing (7)
README.mdsrc/main/java/com/chargebee/v4/client/ChargebeeClient.javasrc/main/java/com/chargebee/v4/telemetry/ChargebeeTelemetryHeaderParser.javasrc/main/java/com/chargebee/v4/telemetry/TelemetryExecutor.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
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- src/test/java/com/chargebee/v4/telemetry/ChargebeeTelemetryHeaderParserTest.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/main/java/com/chargebee/v4/telemetry/TelemetrySupport.java`:
- Around line 327-330: Update both overloads in TelemetrySupport that inspect
requestHeaders so they evaluate the Prefer header values, not just the header
name. Preserve an existing chargebee-telemetry preference, but append
chargebee-telemetry=include when other Prefer values exist without that
directive, including return=minimal.
🪄 Autofix
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: 7594e20a-8e10-48cd-96c5-9d5639eca190
📒 Files selected for processing (2)
src/main/java/com/chargebee/v4/telemetry/TelemetryAttributeKeys.javasrc/main/java/com/chargebee/v4/telemetry/TelemetrySupport.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/chargebee/v4/telemetry/TelemetryAttributeKeys.java
| for (String name : requestHeaders.keySet()) { | ||
| if (name.equalsIgnoreCase(TelemetryAttributeKeys.CHARGEBEE_TELEMETRY_PREFER_HEADER)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check the Prefer value, not only its header name.
Both overloads return when any Prefer header exists. For example, Prefer: return=minimal prevents adding chargebee-telemetry=include, so the server does not return X-Chargebee-Telemetry even when telemetry capture is enabled.
Inspect the existing Prefer value for the Chargebee directive. Append chargebee-telemetry=include when that directive is absent. Preserve an explicit existing Chargebee telemetry preference.
As per path instructions, this is a merge-blocking functionality issue.
Also applies to: 342-345
🤖 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/TelemetrySupport.java` around lines
327 - 330, Update both overloads in TelemetrySupport that inspect requestHeaders
so they evaluate the Prefer header values, not just the header name. Preserve an
existing chargebee-telemetry preference, but append chargebee-telemetry=include
when other Prefer values exist without that directive, including return=minimal.
Source: Path instructions
1 participant
TBA
Adds opt-in Chargebee telemetry support. Clients can request
X-Chargebee-Telemetryresponse data throughPrefer, capture response headers, and export raw and parsed values as typed OpenTelemetry span attributes. Adds RFC 9651 parsing, documentation, client configuration, and comprehensive tests.