-
Notifications
You must be signed in to change notification settings - Fork 82
Add WRITE_XML_DECLARATION property for XML payloads in MI #1903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Added <property name="WRITE_XML_DECLARATION" scope="axis2" value="true"/> to Synapse configuration to include XML declaration in outgoing messages. - Updated Step 1.4 of Grafana-based observability documentation to reflect this change. - This ensures compatibility with systems requiring XML declaration in payloads.
WalkthroughAdded VM doc Step 1.5 to enable XML declarations via Axis2 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
"Hi, could you please review this PR?" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
en/docs/observe-and-manage/setting-up-cloud-native-observability-on-a-vm.md (1)
111-127: Fix markdown code fence error and reorder steps for logical document flow.The new Step 1.5 is inserted between Step 1.4's explanatory text and its code block, disrupting the logical flow. Additionally, the XML code block lacks a closing fence before the TOML block begins, causing markdown rendering errors.
Complete Step 1.4 with its TOML handler configuration first, then add Step 1.5 after it. Ensure the XML code block in Step 1.5 is properly closed with a closing fence.
To enable observability for the WSO2 Integrator: MI servers, add the following Synapse handler to the `deployment.toml` file (stored in the `<MI_HOME>/conf/` folder). +```toml +[[synapse_handlers]] +name="CustomObservabilityHandler" +class="org.wso2.micro.integrator.observability.metric.handler.MetricHandler" +``` + +### Step 1.5 - Enable XML Declaration in Payloads + +If you want your outgoing messages to include the XML declaration, add the following Axis2 property to your deployment.toml: + +```xml +<property name="WRITE_XML_DECLARATION" scope="axis2" value="true"/> +``` + -### Step 1.5 - Enable XML Declaration in Payloads - -If you want your outgoing messages to include the XML declaration, add the following Axis2 property to your Synapse configuration: - -```xml -<property name="WRITE_XML_DECLARATION" scope="axis2" value="true"/> - - -```toml -[[synapse_handlers]] -name="CustomObservabilityHandler" -class="org.wso2.micro.integrator.observability.metric.handler.MetricHandler" -``` After applying the above change, you can start the WSO2 Integrator: MI with the following JVM property:
🧹 Nitpick comments (1)
en/docs/observe-and-manage/setting-up-cloud-native-observability-on-a-vm.md (1)
113-116: Clarify the placement and specificity of the XML property configuration.Line 113 refers to "Synapse configuration" without specifying the
deployment.tomlfile, which is the standard location per Step 1.4. Additionally, clarify whether the XML property should be placed in the same [[synapse_handlers]] section as the CustomObservabilityHandler or in a separate Synapse configuration block.Consider revising to:
If you want your outgoing messages to include the XML declaration, add the following Axis2 property to your Synapse handler configuration in the `deployment.toml` file:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
en/docs/observe-and-manage/setting-up-cloud-native-observability-on-a-vm.md(1 hunks)
Corrected indentation and formatting in YAML configuration for MetricHandler.
Correct indentation for JAVA_OPTS in configuration.
Updated the YAML configuration for Prometheus Agent to improve formatting and clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
en/docs/observe-and-manage/setting-up-cloud-native-observability-in-kubernetes.md(5 hunks)
🔇 Additional comments (3)
en/docs/observe-and-manage/setting-up-cloud-native-observability-in-kubernetes.md (3)
147-151: Straightforward environment variable configuration.The JAVA_OPTS environment variable configuration is clear and enables the Prometheus API endpoint as intended.
160-167: Prometheus annotations are correctly formatted.The annotations for Prometheus service discovery (path, port, and scrape flag) follow the standard Prometheus convention and should work as expected with the Prometheus Agent configuration.
204-204: RBAC resources array format is functionally equivalent.Converting from multi-line resource entries to an inline array is a valid YAML formatting change and should not affect Kubernetes resource behavior.
en/docs/observe-and-manage/setting-up-cloud-native-observability-in-kubernetes.md
Outdated
Show resolved
Hide resolved
en/docs/observe-and-manage/setting-up-cloud-native-observability-in-kubernetes.md
Outdated
Show resolved
Hide resolved
Updated YAML configuration for WSO2 Integrator: MI to correct indentation and structure for synapseHandlers, deployment envs, and deployment annotations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
en/docs/observe-and-manage/setting-up-cloud-native-observability-in-kubernetes.md (2)
132-138: MetricHandler class path uses incorrect package format (duplicate of prior review feedback).Line 135 shows
org.wso2.micro_integrator.observability.metric.handler.MetricHandlerwith an underscore in the package name. The correct path should use dots:org.wso2.micro.integrator.observability.metric.handler.MetricHandler. This exact issue was flagged in a previous review; please ensure the fix is applied.
458-458: Invalid regex syntax for named capture group (duplicate of prior review feedback).The regex uses
(<message>.*)which is not valid Oniguruma named capture group syntax. This was flagged in a previous review. The correct syntax is(?<message>.*). The current pattern will attempt to match the literal text<message>instead of creating a named capture group, which will not extract the message field correctly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
en/docs/observe-and-manage/setting-up-cloud-native-observability-in-kubernetes.md(5 hunks)
en/docs/observe-and-manage/setting-up-cloud-native-observability-in-kubernetes.md
Outdated
Show resolved
Hide resolved
|
Hi @SanojPunchihewa, |
|
Hi, I’ve addressed the review comments by:
Could you please re-review the latest changes? |
Summary
WRITE_XML_DECLARATION=trueto the Synapse configuration.Problem Statement
Some external systems require an explicit XML declaration (for example,
<?xml version="1.0" encoding="UTF-8"?>) in XML payloads.By default, WSO2 Micro Integrator does not include this declaration, which can cause integration failures.
Solution
Impact
Documentation
The following documentation was updated:
setting-up-cloud-native-observability-on-a-vm.mdsetting-up-cloud-native-observability-in-kubernetes.mdTesting
Security Considerations
Backward Compatibility
Related Issues / PRs
N/A