Introduce support for liveness check for MSSQL CDC listener#1170
Introduce support for liveness check for MSSQL CDC listener#1170gayaldassanayake merged 15 commits intoballerina-platform:mainfrom
Conversation
gayaldassanayake
requested review from
aashikam,
niveathika and
shafreenAnfar
as code owners
|
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:
📝 WalkthroughWalkthroughBumps mssql and related artifacts to 1.16.3, adds Avro/Kafka/Confluent packages and Gradle deps, changes CdcListener config from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina/cdc_listener.bal`:
- Around line 52-56: The code unconditionally sets
listenerConfigs["livenessInterval"] = config.livenessInterval which may insert
the empty tuple () when livenessInterval is absent; update the logic to guard
the assignment by checking that config.livenessInterval is not the empty value
(e.g., if config.livenessInterval != () { listenerConfigs["livenessInterval"] =
config.livenessInterval; }) before calling self.config =
listenerConfigs.cloneReadOnly(), so only present livenessInterval values are
added.
| map<anydata> listenerConfigs = { | ||
| ...debeziumConfigs | ||
| }; | ||
| listenerConfigs["livenessInterval"] = config.livenessInterval; | ||
| self.config = listenerConfigs.cloneReadOnly(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find MsSqlListenerConfiguration definition and livenessInterval usage
rg -n "MsSqlListenerConfiguration" -g '*.bal' -C 5Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 1585
🏁 Script executed:
#!/bin/bash
# Search for livenessInterval declarations and usages
rg -n "livenessInterval" -g '*.bal' -B 2 -A 2Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 409
🏁 Script executed:
#!/bin/bash
# Check the cdc_listener.bal file around the target lines
cat -n ballerina/cdc_listener.bal | sed -n '40,70p'Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 2016
🏁 Script executed:
#!/bin/bash
# Search for cdc:ListenerConfiguration in the codebase
rg -n "ListenerConfiguration" -g '*.bal' -B 2 -A 8Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 1862
🏁 Script executed:
#!/bin/bash
# Check types.bal for the full type definitions
cat -n ballerina/types.balRepository: ballerina-platform/module-ballerinax-mssql
Length of output: 10105
🏁 Script executed:
#!/bin/bash
# Search for any cdc-related files in the repository
find . -name "*cdc*" -type f 2>/dev/null | head -20Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 162
🏁 Script executed:
#!/bin/bash
# Search for imports and usage of livenessInterval across the codebase
rg -n "livenessInterval" -g '*.bal' -B 3 -A 3Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 511
🏁 Script executed:
#!/bin/bash
# Check if there are any Ballerina lock files or dependencies
fd -e "lock" -e "Ballerina.lock" 2>/dev/nullRepository: ballerina-platform/module-ballerinax-mssql
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Find and read Ballerina.toml
fd -name "Ballerina.toml" -o -name "*.toml" 2>/dev/null | head -5Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Read the Ballerina.toml file
cat Ballerina.toml 2>/dev/null || cat ballerina/Ballerina.toml 2>/dev/nullRepository: ballerina-platform/module-ballerinax-mssql
Length of output: 809
🏁 Script executed:
#!/bin/bash
# Check how config parameter is passed and used - look for calls to init function
rg -n "init\(" -g '*.bal' -B 2 -A 5Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 6113
🏁 Script executed:
#!/bin/bash
# Search for test files that use MsSqlListener initialization
find . -name "*test*.bal" -o -name "*Test.bal" | head -10Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 510
🏁 Script executed:
#!/bin/bash
# Look for test files and examples
fd -name "*test*" -type f -g "*.bal" 2>/dev/null | head -20Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Search for how the listener is instantiated in tests
rg -n "new MsSqlListener" -g '*.bal' -B 3 -A 5Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Check the listener test file
cat -n ballerina/tests/listener_tests.bal | head -100Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 3957
🏁 Script executed:
#!/bin/bash
# Search for MsSqlListener in test files
rg -n "MsSqlListener" -g '*.bal' -B 2 -A 5Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 4091
🏁 Script executed:
#!/bin/bash
# Get the full cdc_listener.bal file to see complete context
cat -n ballerina/cdc_listener.balRepository: ballerina-platform/module-ballerinax-mssql
Length of output: 5188
🏁 Script executed:
#!/antml:thinking
Looking at the test file, I can see that the `getDummyMsSqlListener()` function creates a listener without passing `livenessInterval`. This suggests it's optional. Let me check the full init function to understand the context better.Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 154
🏁 Script executed:
#!/bin/bash
# Search for any comments or documentation about config handling
rg -n "config\[" -g '*.bal' -B 2 -A 2Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Look for patterns of guarding assignments in similar codebases
rg -n "if .* != \(\)" -g '*.bal' -B 1 -A 1Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 68
🌐 Web query:
cdc:ListenerConfiguration livenessInterval ballerina optional
💡 Result:
In the current Ballerina CDC module (ballerinax/cdc), cdc:ListenerConfiguration does not have a livenessInterval field (so there’s nothing to mark “optional” vs “required”). The base record only includes:
engineName(has a default)internalSchemaStorage(has a default)offsetStorage(has a default)options(has a default)
So livenessInterval is simply not a supported config property for cdc:ListenerConfiguration (as of ballerinax/cdc latest docs). [1]
Source: Ballerina Central API docs for cdc:ListenerConfiguration. [1]
[1] (central.ballerina.io)
Citations:
🌐 Web query:
Ballerina cdc module ListenerConfiguration field definitions
💡 Result:
In ballerinax/cdc, ListenerConfiguration is a closed record with these fields:
engineName: string(default"ballerina-cdc-connector") — Name of the CDC engine. [1]internalSchemaStorage: FileInternalSchemaStorage|KafkaInternalSchemaStorage(default{}) — Internal schema history configuration. [1]offsetStorage: FileOffsetStorage|KafkaOffsetStorage(default{}) — Offset storage configuration. [1]options: Options(default{}) — Additional CDC engine options. [1]
Source:
[1] Ballerina Central – ballerinax/cdc module docs (ListenerConfiguration record): (central.ballerina.io)
Citations:
🏁 Script executed:
#!/bin/bash
# Check if the code compiles and get more context around config handling
rg -n "config\." -g 'cdc_listener.bal' -B 1 -A 1Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 2673
🏁 Script executed:
#!/antml:thinking
Based on the web search results, cdc:ListenerConfiguration does NOT have a livenessInterval field. But the code is accessing config.livenessInterval. This is strange. Let me check if maybe the record allows additional fields (open record) or if there's a recent addition.
Actually, looking at the record definition again:Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 444
🏁 Script executed:
#!/an/bash
# Check if there are type definitions or imports that might add livenessInterval
cat -n ballerina/cdc_listener.bal | grep -E "import|type"Repository: ballerina-platform/module-ballerinax-mssql
Length of output: 115
Guard optional livenessInterval before insertion.
Since livenessInterval is optional in the configuration, when not provided it defaults to (). Unconditionally inserting () into the config map before passing to native config parsing may cause issues. Guard the assignment:
Suggested fix
- listenerConfigs["livenessInterval"] = config.livenessInterval;
+ if config.livenessInterval != () {
+ listenerConfigs["livenessInterval"] = config.livenessInterval;
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ballerina/cdc_listener.bal` around lines 52 - 56, The code unconditionally
sets listenerConfigs["livenessInterval"] = config.livenessInterval which may
insert the empty tuple () when livenessInterval is absent; update the logic to
guard the assignment by checking that config.livenessInterval is not the empty
value (e.g., if config.livenessInterval != () {
listenerConfigs["livenessInterval"] = config.livenessInterval; }) before calling
self.config = listenerConfigs.cloneReadOnly(), so only present livenessInterval
values are added.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
ballerina/tests/listener_tests.bal (2)
203-207: Consider resetting event counters before the test for isolation.The global mutable counters (
createEventCount,updateEventCount, etc.) are initialized to 0 at module load but never reset between test runs. If tests run in a different order or if future tests reuse these counters, assertions may fail unexpectedly.🛡️ Suggested fix to reset counters at test start
Add counter resets at the beginning of
testCdcListenerEvents:function testCdcListenerEvents() returns error? { + // Reset event counters for test isolation + createEventCount = 0; + updateEventCount = 0; + deleteEventCount = 0; + readEventCount = 0; + onErrorCount = 0; + CdcListener testListener = new ({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina/tests/listener_tests.bal` around lines 203 - 207, The module-level counters createEventCount, updateEventCount, deleteEventCount, readEventCount, and onErrorCount are never reset between tests; update the start of testCdcListenerEvents to explicitly reset each counter to 0 (set createEventCount = 0; updateEventCount = 0; deleteEventCount = 0; readEventCount = 0; onErrorCount = 0;) so the test runs with a clean state and remains isolated from other tests.
219-220: Minor: Unconventional comma placement.The trailing comma on a separate line works but is unusual. Consider moving it to the end of the previous line for conventional formatting.
🔧 Suggested formatting
database: { username: cdcUsername, password: cdcPassword, port: cdcPort, - databaseNames: cdcDatabase - , - includedTables: ["store_db.dbo.products", "store_db.dbo.vendors"] + databaseNames: cdcDatabase, + includedTables: ["store_db.dbo.products", "store_db.dbo.vendors"] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina/tests/listener_tests.bal` around lines 219 - 220, The trailing comma is placed on its own line after the previous object/field; move the comma to the end of the previous line so the array/property line reads `includedTables: ["store_db.dbo.products", "store_db.dbo.vendors"],` (or place the comma immediately after the previous element) to follow conventional formatting for the includedTables field.ballerina/tests/listener_liveness_tests.bal (1)
25-35: Consider a helper to build the commonCdcListenerconfig.
The listener config is repeated across four tests. A small factory/helper (optionally takinglivenessInterval) would reduce duplication and make future config changes less error-prone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina/tests/listener_liveness_tests.bal` around lines 25 - 35, Extract the duplicated CdcListener construction into a small factory function (e.g., buildCdcListenerConfig or createCdcListener) that returns a CdcListener instance or its config object and accepts an optional livenessInterval parameter; move the common fields (database: username, password, port, databaseNames and options: snapshotMode = cdc:NO_DATA) into that helper and use it in place of the repeated construction (currently assigned to mssqlListener in each test), then update the four tests to call the helper (passing livenessInterval when needed) to remove duplication and centralize future config changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ballerina/tests/listener_liveness_tests.bal`:
- Around line 25-35: Extract the duplicated CdcListener construction into a
small factory function (e.g., buildCdcListenerConfig or createCdcListener) that
returns a CdcListener instance or its config object and accepts an optional
livenessInterval parameter; move the common fields (database: username,
password, port, databaseNames and options: snapshotMode = cdc:NO_DATA) into that
helper and use it in place of the repeated construction (currently assigned to
mssqlListener in each test), then update the four tests to call the helper
(passing livenessInterval when needed) to remove duplication and centralize
future config changes.
In `@ballerina/tests/listener_tests.bal`:
- Around line 203-207: The module-level counters createEventCount,
updateEventCount, deleteEventCount, readEventCount, and onErrorCount are never
reset between tests; update the start of testCdcListenerEvents to explicitly
reset each counter to 0 (set createEventCount = 0; updateEventCount = 0;
deleteEventCount = 0; readEventCount = 0; onErrorCount = 0;) so the test runs
with a clean state and remains isolated from other tests.
- Around line 219-220: The trailing comma is placed on its own line after the
previous object/field; move the comma to the end of the previous line so the
array/property line reads `includedTables: ["store_db.dbo.products",
"store_db.dbo.vendors"],` (or place the comma immediately after the previous
element) to follow conventional formatting for the includedTables field.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1170 +/- ##
============================================
- Coverage 82.42% 82.25% -0.18%
+ Complexity 193 192 -1
============================================
Files 19 19
Lines 922 924 +2
Branches 159 159
============================================
Hits 760 760
- Misses 89 90 +1
- Partials 73 74 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina/Dependencies.toml`:
- Line 462: The dependency entry for ballerinax/mssql currently sets version =
"1.16.3" which does not exist; update that version string to "1.16.2" (the
latest released version) by replacing "1.16.3" with "1.16.2" in the
Dependencies.toml dependency block for ballerinax/mssql so the project
references the valid release.
- Line 55: Dependencies specify crypto version "2.9.3" which includes breaking
post‑quantum OID changes for MLDSA and MLKEM; search the repo for any usage of
the "crypto" module and explicit references to MLDSA or MLKEM (key generation,
key parsing, encryption/decryption, verify functions) and confirm you do not
rely on keys or ciphertext produced by older crypto releases; if you do, either
regenerate keys/ciphertexts under crypto 2.9.3 or pin the crypto dependency to
the prior compatible version, and update any key-loading/parsing logic to handle
the new OIDs (or add migration code) so MLDSA/MLKEM assets remain usable.
- Around line 426-435: The Dependencies.toml shows version mismatch risks:
confluent.cavroserdes (1.0.2) and confluent.cregistry (0.4.3) target Ballerina
2201.11.0 while kafka 4.6.3 and MSSQL target 2201.12.0; verify that Ballerina
2201.12.0 is compatible with the 2201.11.0 constraints or update
confluent.cavroserdes and confluent.cregistry to releases that explicitly target
2201.12.0 (or align all packages to a single compatible Ballerina version). Also
confirm redistribution compliance for confluent.cavroserdes/confluent.cregistry
(Apache-2.0) by including upstream LICENSE and NOTICE files from Confluent
Schema Registry and Kafka clients in the release artifacts and repository.
Finally, ensure kafka 4.6.3 release notes or a breaking-change summary are
documented for consumers.
- Around line 412-420: The Dependencies.toml lists version = "1.2.0" which
doesn't exist for ballerinax/cdc and the observe dependency uses org =
"ballerinai" (typo); update the cdc dependency version to the published "1.1.0"
(or explicitly note if 1.2.0 is intentionally unreleased) and correct the
observe entry to org = "ballerina"; specifically edit the version = "1.2.0" line
and the dependency objects referencing ballerinax/cdc and {org = "ballerinai",
name = "observe"} (change to {org = "ballerina", name = "observe"}) accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gradle.properties`:
- Line 33: The gradle property stdlibCdcVersion is pinned to a timestamped
snapshot (stdlibCdcVersion=1.2.0-20260213-180200-2268a4d); replace that value
with the intended stable release (for example "1.2.0") before merging so the
project uses a reproducible, released artifact—update the stdlibCdcVersion entry
in gradle.properties accordingly and commit the change.
---
Duplicate comments:
In `@ballerina/Dependencies.toml`:
- Around line 409-420: The Dependencies.toml entry for package
"ballerinax"/"cdc" currently lists version "1.2.0" which is unpublished; change
that version to the published "1.1.0" in the [[package]] block for
org="ballerinax" name="cdc" and then regenerate the lockfile (re-run whatever
tool produces this auto-generated Dependencies.toml) so the file and lockfile
are consistent; ensure any upstream source dependency (the project's source
dependency configuration) that led to this generated entry is updated to point
at 1.1.0 before regenerating.
- Around line 459-462: The Dependencies.toml entry for org "ballerinax" name
"mssql" currently pins version "1.16.3", which appears unpublished; update the
source dependency to pin "1.16.2" (or publish 1.16.3) and then regenerate the
project's lockfile so dependency resolution succeeds; locate the package block
with org="ballerinax" name="mssql" version="1.16.3", change the version to
"1.16.2" in the source manifest, and run the dependency/lockfile generation
command used in the project to produce an updated Dependencies.toml/lockfile.
- Around line 426-435: The Dependencies.toml entry for package
"confluent.cavroserdes" version "1.0.2" (and the referenced
"confluent.cregistry" 0.4.3) declare compatibility with Ballerina 2201.11.0
while this repo targets 2201.12.0; verify those packages run correctly on
2201.12.0 or update the dependency versions to releases that explicitly support
2201.12.0. Specifically, check the central metadata for "confluent.cavroserdes"
and "confluent.cregistry" and either (a) bump the versions in Dependencies.toml
to the 2201.12.0-compatible releases if available, or (b) add a
comment/validation in the repo to confirm runtime compatibility with Ballerina
2201.12.0.
4 participants
Purpose
$ subject
Examples
Checklist
Overview
Adds liveness check support to the MSSQL CDC listener to enable runtime health monitoring of CDC connections. The change updates listener configuration handling, adds liveness-focused tests, adjusts existing tests for MSSQL table naming, and includes dependency/build updates.
Key changes
Implementation
Tests
Build and dependencies
Impact and notes