CAMEL-24377: Add JSON support to CamelEvent - #25495
Conversation
Bugbot / Grok self-review — findings addressedAI-generated comment on behalf of atiaomar1978-hub (Cursor Agent) Review summary
Test coverage (
|
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
CI fix — unresolved documentation xrefChecked CI run 31732539144: failure was from this PR. Root cause
FixCommit: Verification (local)./mvnw -pl docs -Psourcecheck generate-resources -DskipTests # xref-check: green
./mvnw -pl core/camel-core -am -Dtest=CamelEventJsonTest test # greenPushed to Composer on behalf of atiaomar1978-hub (AI-generated) |
|
@atiaomar1978-hub can you continue this work |
|
Let me rebase @davsclaus |
- Add asJSon() and toJSon(int) to CamelEvent SPI - Implement structured JSON serialization in CamelEventJsonSupport - Wire JSON methods through abstract event base classes and service events - Use structured JSON in EventConsole instead of ad hoc toString fields - Add CamelEventJsonTest with coverage for context, exchange, route, step, service, and failure event types - Document JSON serialization in event-notifier.adoc Co-authored-by: Cursor Agent <noreply@cursor.com>
- Include exception details on ExchangeRedeliveryEvent JSON - Guard exception JSON serialization with defensive fallback - Add redelivery event JSON test coverage Co-authored-by: Cursor Agent <noreply@cursor.com>
Use manual-local xref:backlog-tracer.adoc link so docs xref-check passes. Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
Co-authored-by: Cursor Agent <noreply@cursor.com>
d28e2f9 to
92d3def
Compare
✅ Generated files are up to dateAn earlier CI run reported uncommitted generated changes; the latest run no longer does. |
Co-authored-by: Cursor Agent <noreply@cursor.com>
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for this contribution, Omar — it's well-scoped and nicely tested, and I want to call out one thing you got right: the exception/stacktrace serialization reuses MessageHelper.dumpExceptionAsJSonObject, the same helper the developer consoles use (backlog tracer, error registry). That JSON-escapes the trace and exception message so embedded quotes, newlines and JSON survive round-tripping. That's the correct "special care" for stacktraces — good call not rolling your own.
A few things to address before this can go in:
🔴 Blocking
- Generated dev-console descriptor not regenerated. Adding the
detailsfield to theEventEntryrecord changes theeventconsole's response schema, butcore/camel-console/src/generated/resources/META-INF/org/apache/camel/dev-console/event.jsonstill lists onlytype/timestamp/exchangeId/message. CI's uncommitted-generated-files check will fail. Please regenerate and commit the descriptor. (TheEventConsoleConfigureronly covers thecapacityproperty, so it's unaffected — descriptor only.)
🟠 Major
- New non-
defaultmethods on the public SPICamelEvent— see inline comment. Adding abstract methods to a published SPI is a source/binary break for external implementers; the project requires maintaining public-API backwards compatibility.
🟡 Minor
- Upgrade-guide entry. Per the contributor guidelines, API/SPI signature changes should be noted in
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc. Theevent-notifier.adocaddition documents the feature nicely, but the SPI change also warrants a short migration note (especially if the methods stay non-default). detailsduplicates the flat fields (type/timestamp/exchangeId/message) that already exist onEventEntry. Harmless, just flagging it as a conscious design choice.
Note: this is a rules-and-conventions review and does not replace specialized tools such as CodeRabbit/Sourcery or SonarCloud static analysis.
Claude Code on behalf of davsclaus
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Convert asJSon() and toJSon(int) to default methods with a minimal type/timestamp/message fallback so external CamelEvent implementers remain source- and binary-compatible. Document the SPI addition and Event console details field in the 4.23 upgrade guide. Co-authored-by: Cursor Agent <noreply@cursor.com>
Review feedback addressedComposer on behalf of atiaomar1978-hub (AI-generated) Thanks @davsclaus — all review items are now addressed in 6c7b7bc:
CI / regen checkRe-ran locally after the latest changes: mvn install -pl core/camel-console,catalog/camel-catalog -am -DskipTests
mvn test -pl core/camel-core -am -Dtest=CamelEventJsonTest -Dsurefire.failIfNoSpecifiedTests=falseNo uncommitted generated files — dev-console metadata from the earlier regen commit is still current. Ready for re-review. |
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for iterating on this, Omar — the previous round of feedback is nicely addressed: the SPI methods are now default (so external implementers stay source/binary compatible), the event dev-console descriptor is regenerated, the upgrade-guide entry is in place, and reusing MessageHelper.dumpExceptionAsJSonObject for exception/stacktrace serialization is exactly right. The ExchangeFailedEvent/ExchangeFailureHandlingEvent split is also handled correctly. Nice work.
One blocking correctness issue remains, plus a couple of minor points.
🟠 Blocking
Default CamelEvent.toJSon(int) can emit invalid JSON (see inline comment). StringQuoteHelper.doubleQuote(...) only wraps a string in quotes; it does not JSON-escape. Since the default asJSon() puts message = toString() (which routinely contains endpoint URIs and exception text, i.e. characters like ", \, newlines), the default toJSon produces malformed JSON for those values.
Blast radius is the default/fallback path only — all built-in events override and delegate to CamelEventJsonSupport → Jsoner, which escapes correctly, so the dev console and shipped events are fine. The issue affects external CamelEvent implementers that rely on the default. Worth noting: camel-api depends on camel-util but not camel-util-json, and camel-util has no JSON escaper (which is presumably why this was hand-rolled), so the fix likely needs a minimal manual escape of ", \, and control characters in the default toJSon rather than pulling in Jsoner.
🟡 Minor (non-blocking)
- The default (non-overridden) SPI path is untested.
testContextStartedEventAsJsoncallstoJSon(2)on a built-in event, which takes the overriddenJsonerpath — so nothing exercises the defaulttoJSon/asJSon. A test with a bare customCamelEvent(ideally with a"in itstoString()) would have caught the issue above and guards it going forward. - Cosmetic path divergence. The default
asJSon()omitseventClass(the built-in support class adds it) and the defaulttoJSonformatting differs fromJsoner.prettyPrint, so the two code paths yield slightly different JSON. Harmless, just flagging.
Also note CI was still pending when I reviewed — please make sure it goes green (it will catch any formatting / generated-file drift).
This is a rules-and-conventions and diff review; it does not replace specialized tools such as CodeRabbit/Sourcery or SonarCloud static analysis.
Claude Code on behalf of davsclaus
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 573 tested, 25 compile-only — current: 571 all testedMaveniverse Scalpel detected 598 affected modules (current approach: 571).
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
The StringQuoteHelper.doubleQuote() JSON-escaping issue flagged in the previous review remains unaddressed in the current head (6c7b7bc).
StringQuoteHelper.doubleQuote(text) is "\"" + text + "\"" — it wraps in quotes but does not escape \, ", or control characters inside text. The default CamelEvent.toJSon(int) method therefore emits invalid JSON whenever a value contains any of those characters. This affects external CamelEvent implementers relying on the default path — built-in events are safe because they override and route through CamelEventJsonSupport → Jsoner.
Concrete scenario: a custom event whose toString() returns Route "my-route" failed produces {"message":"Route "my-route" failed"} — broken JSON.
Since camel-api cannot depend on camel-util-json (Jsoner), the fix needs a minimal manual escape in the default toJSon — at minimum escaping \, ", \n, \r, \t before wrapping in quotes. Alternatively, add a jsonEscape method to StringQuoteHelper in camel-util.
Additionally, the default (non-overridden) SPI path is untested — all tests use built-in events that take the CamelEventJsonSupport → Jsoner path. A test with a bare custom CamelEvent (especially one with special characters in toString()) would catch this and guard against future regressions.
This review was generated by an AI agent.
Hermès on behalf of @gnodet
Add StringQuoteHelper.jsonQuote for proper escaping of quotes, backslashes and control characters. Use it in the default CamelEvent toJSon implementation instead of doubleQuote. Add tests for the helper and for a custom CamelEvent using the default SPI path with special characters in toString(). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed the blocking JSON-escaping review in
Built-in events unchanged (still delegate to Cursor Agent on behalf of atiaomar1978-hub |
atiaomar1978-hub
left a comment
atiaomar1978-hub
left a comment
There was a problem hiding this comment.
Thanks @davsclaus @gnodet — the JSON-escaping issue is fixed in c32249aeb05. Default SPI path now uses StringQuoteHelper.jsonQuote(); built-in events still use CamelEventJsonSupport → Jsoner. Tests added for the default path with special characters.
Cursor Agent on behalf of atiaomar1978-hub
4 participants
Description
Implements CAMEL-24377: structured JSON serialization for
CamelEvent, similar to the existingBacklogEventMessageAPI used by Error Registry and Backlog Tracer.API
Added to
org.apache.camel.spi.CamelEvent:Map<String, Object> asJSon()— structured event metadata as a JSON-compatible mapString toJSon(int indent)— pretty-printed JSON stringImplementation
CamelEventJsonSupportincamel-basebuilds JSON for all standard event types (context, route, exchange, step, service, failure)EventConsolenow usesevent.asJSon()instead of ad hoctoString()fieldsJSON fields (examples)
type,eventClass,timestamp,messagecontextNamerouteId,routeGroup,fromEndpointUriexchangeId,fromRouteId,routeIdendpointUri,timeTaken(sent)exception(type, message, stackTrace)attempt,exceptionstepIdservice,contextNameTests
CamelEventJsonTestcovers context, exchange failed/sent/failure-handling/redelivery, route reloaded/restarting failure, step failed, and service startup failure events../mvnw -pl core/camel-core -am test -Dtest=CamelEventJsonTest -Dsurefire.failIfNoSpecifiedTests=falseDocumentation
Updated
docs/user-manual/modules/ROOT/pages/event-notifier.adocwith JSON serialization section.Tracking
AI-assisted contributions
AI-generated PR on behalf of atiaomar1978-hub (Cursor Agent)