MongoDB Exchange Store YAML Example #2528
Conversation
…or missing classes and shorten output
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds explicit NoClassDefFoundError handling to YAML instantiation in GenericYamlParser, introduces a MongoDB-backed exchange-store example with an Changes
Sequence Diagram(s)(Skipped — changes are localized error handling, example configuration, and documentation; no new multi-component sequential control flow.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 4
🧹 Nitpick comments (2)
annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java (2)
193-205: Consider handling null message from cause.If
e.getCause().getMessage()returnsnull, the error message will contain "Missing class: null", which may be confusing. Consider falling back to the cause's class name or a default message.Also, note the minor spacing inconsistency on line 201:
throw new ParsingException(e,node);— add a space after the comma for consistency with the project's style.🔎 Suggested improvement
catch (NoClassDefFoundError e) { if (e.getCause() != null) { - var missingClass = e.getCause().getMessage(); // TODO: Better use ExceptionUtil.getRootCause() but it isn't visible in annot. + var cause = e.getCause(); + var missingClass = cause.getMessage() != null ? cause.getMessage() : cause.getClass().getName(); var msg = "Could not create bean with class: %s\nMissing class: %s\nConfiguration: %s".formatted(clazz, missingClass, node); log.error(msg); throw new ParsingException(msg, node); // TODO: Cause we know the reason, shorten output. } - throw new ParsingException(e,node); + throw new ParsingException(e, node); }
196-199: TODO comments indicate deferred improvements.The TODO comments here align with the roadmap item about shortening error messages. Consider creating issues to track these improvements:
- Refactoring to use
ExceptionUtil.getRootCause()once it's accessible from theannotmodule- Shortening the error output when the cause is known
Would you like me to open issues to track these TODO items?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.javadistribution/examples/extending-membrane/mongo-exchange-store/README.mddistribution/examples/extending-membrane/mongo-exchange-store/apis.yamldocs/ROADMAP.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Automated tests
- GitHub Check: Analyze (java)
🔇 Additional comments (2)
distribution/examples/extending-membrane/mongo-exchange-store/README.md (2)
3-4: LGTM!The updated description clearly conveys the MongoDB persistence benefits and use cases.
23-23: LGTM!The configuration step correctly references the new
apis.yamlfile, aligning with the YAML-first configuration approach.
annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java
Outdated
Show resolved
Hide resolved
distribution/examples/extending-membrane/mongo-exchange-store/README.md
Outdated
Show resolved
Hide resolved
…n formatting, and update MongoDB driver instructions in README
…ple' into mongodb-exchange-store-yaml-example
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
distribution/examples/extending-membrane/mongo-exchange-store/README.md (1)
20-23: Fix list indentation for proper Markdown formatting.The nested list items on lines 20 and 23 are indented with 2 spaces but should start at column 0 for proper Markdown list formatting.
🔎 Proposed fix
1. **Download MongoDB Driver:** - - Choose from: - [https://repo1.maven.org/maven2/org/mongodb/mongodb-driver-sync/](https://repo1.maven.org/maven2/org/mongodb/mongodb-driver-sync/) - a recent version. 5.X is recommended. - - Place it in the `lib` directory of your Membrane installation. +- Choose from: + [https://repo1.maven.org/maven2/org/mongodb/mongodb-driver-sync/](https://repo1.maven.org/maven2/org/mongodb/mongodb-driver-sync/) + a recent version. 5.X is recommended. +- Place it in the `lib` directory of your Membrane installation.
🧹 Nitpick comments (1)
distribution/examples/extending-membrane/mongo-exchange-store/README.md (1)
20-23: Consider adding MongoDB compatibility notes.While the generic "5.X is recommended" approach is more maintainable than pinning to a specific version, consider adding:
- Minimum MongoDB Server version requirement (driver 5.x requires Server ≥4.2)
- Brief note about potential breaking changes when upgrading between 5.x versions
- Optional: Maven/Gradle dependency snippets as an alternative to manual JAR downloads for easier dependency management
This would help users avoid compatibility issues and streamline setup.
📝 Example addition
- Choose from: [https://repo1.maven.org/maven2/org/mongodb/mongodb-driver-sync/](https://repo1.maven.org/maven2/org/mongodb/mongodb-driver-sync/) a recent version. 5.X is recommended. - **Note:** MongoDB Java driver 5.x requires MongoDB Server ≥4.2. Review release notes for breaking changes between versions. - Place it in the `lib` directory of your Membrane installation. Alternatively, add to your `pom.xml`: ```xml <dependency> <groupId>org.mongodb</groupId> <artifactId>mongodb-driver-sync</artifactId> <version>5.6.0</version> </dependency></details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 62bc690d51424973433e938636015bb0df6a5045 and f1276a7c9fbe83bca40a3cabdbbccba67bf9e0a0. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java` * `distribution/examples/extending-membrane/mongo-exchange-store/README.md` </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (1)</summary> <details> <summary>📚 Learning: 2025-09-22T19:49:29.473Z</summary>Learnt from: predic8
Repo: membrane/api-gateway PR: 2156
File: pom.xml:0-0
Timestamp: 2025-09-22T19:49:29.473Z
Learning: In the Membrane API Gateway project, jackson-annotations is pinned to version "2.20" instead of using ${jackson.version} (2.20.0) because version 2.20.0 is not available in Maven repository yet for jackson-annotations, while it is available for jackson-core and jackson-databind. This version inconsistency is intentional and necessary due to different release schedules between Jackson modules.**Applied to files:** - `annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java` </details> </details><details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>distribution/examples/extending-membrane/mongo-exchange-store/README.md</summary> 20-20: Unordered list indentation Expected: 0; Actual: 2 (MD007, ul-indent) --- 23-23: Unordered list indentation Expected: 0; Actual: 2 (MD007, ul-indent) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary> * GitHub Check: Analyze (java) </details> <details> <summary>🔇 Additional comments (3)</summary><blockquote> <details> <summary>annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java (1)</summary><blockquote> `190-202`: **LGTM! Improved error messages for missing class dependencies.** The specific `NoClassDefFoundError` handling provides clearer diagnostics when bean instantiation fails due to missing classes, which aligns well with the PR objective to improve YAML parsing error messages. </blockquote></details> <details> <summary>distribution/examples/extending-membrane/mongo-exchange-store/README.md (2)</summary><blockquote> `3-4`: **LGTM! Clear description of MongoDB-backed persistence.** The updated description effectively communicates the value of persistent storage and post-restart inspection capabilities. --- `25-25`: **LGTM! Shift to YAML configuration.** The change from `proxies.xml` to `apis.yaml` aligns with the PR's YAML-driven configuration approach and the new `apis.yaml` file added to this example. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java
Show resolved
Hide resolved
|
This pull request needs "/ok-to-test" from an authorized committer. |
|
/ok-to-test |
- Added `apis.yaml` with OpenTelemetry plugin configuration for distributed tracing in Membrane. - Improved `OpenTelemetryInterceptor` logging with actionable advice for missing `jul-to-slf4j` JAR. - Updated tests in `MethodSetterTest` to enhance coverage for scalar coercion. - Refactored `MethodSetter` to improve scalar type coercion logic and exception clarity. - Simplified OpenTelemetry example in README; replaced XML example with YAML, added base instructions, and updated trace output.
* extend: Add OpenTelemetry example in YAML (#2528) - Added `apis.yaml` with OpenTelemetry plugin configuration for distributed tracing in Membrane. - Improved `OpenTelemetryInterceptor` logging with actionable advice for missing `jul-to-slf4j` JAR. - Updated tests in `MethodSetterTest` to enhance coverage for scalar coercion. - Refactored `MethodSetter` to improve scalar type coercion logic and exception clarity. - Simplified OpenTelemetry example in README; replaced XML example with YAML, added base instructions, and updated trace output. * refactor: rename test method in `MethodSetterTest` and fix typos in OpenTelemetry example documentation
enhance: improve YAML parsing error handling with specific messages f…
…or missing classes and shorten output
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.