Skip to content

Commit 12db0c5

Browse files
github-merge-queue[bot]Anish Khanzodeakhanzodedd-agent-integrations-bot[bot]
committed
[voltdb] Switch to native VoltDB Python client (#24714)
* [voltdb] Switch to native VoltDB Python client Migrate from the deprecated HTTP/JSON interface to the native voltdbclient Python package, which speaks the binary protocol on the VoltDB client port. Instance configuration now takes 'host' and 'port' (default 21212) instead of 'url'. TLS is configured via 'use_ssl' and 'ssl_config_file'. The legacy 'url' option is still accepted with a deprecation warning so existing deployments keep working unchanged: the host is parsed from the URL and the default native client port is used. Statistics columns are now resolved by name against the VoltDB response metadata so the check tolerates VoltDB releases that add or drop columns to @statistics outputs. * Rename changelog fragment to match PR #23667 * [voltdb] Document SSL properties file format in README * [voltdb] Document password_hashed regression and secrets backend * [voltdb] Default procedure_timeout to 60s Addresses Codex review feedback: the previous code path passed procedure_timeout=None to FastSerializer when the option was omitted, which means 'wait indefinitely'. The old HTTP integration had a 10s default timeout, so leaving the native client with no default is a regression that could block a check run forever on a hung procedure. Default to 60s (matching the example we ship in conf.yaml.example). Setting procedure_timeout to 0 or any non-positive number restores the 'wait indefinitely' behavior for users who explicitly want it. * [voltdb] Fix CI lint and add voltdbclient license entry The CI 'Lint' step failed because ruff lint/isort is invoked from inside the integration directory with --config ../pyproject.toml, where the local package (datadog_checks.voltdb) is treated as first-party and needs its own import block. Running ruff from the repo root (as I did locally) didn't catch this. Apply the isort fix that ruff --fix produces under that working directory. Also regenerated LICENSE-3rdparty.csv via 'ddev validate licenses --sync' to add voltdbclient's MIT license entry, which CI flagged as missing. The other line changes in that file are ddev's current copyright-parser output for existing entries (no upstream changes), kept to satisfy the validator. * [voltdb] Restore HTTP/JSON transport for VMC users The previous commits replaced the HTTP/JSON transport with the native binary client. As feedback noted, some operators connect to VoltDB through the VoltDB Management Center (VMC) rather than directly to database nodes — those deployments need the HTTP transport. Make the transport choice config-driven instead of removing one of them: setting 'url' selects the HTTP client (talks to VMC), and setting 'host' selects the native binary client. Everything else (auth, statistics components, custom queries, tags) is shared. This restores full backwards compatibility for existing 'url'-based configs along with all their HTTP-only options (password_hashed, proxy, tls_cert / tls_ca_cert / tls_verify, headers, etc.) via the instances/http template, and reclassifies the changelog entry from 'changed' to 'added' since nothing is being removed. Common response shape: HttpClient wraps the JSON response so the check code reads response.tables[i].columns[j].name / .tuples on both paths, with no mode-specific branching in _execute_query_raw. New unit test 'test_http_mode_end_to_end' patches requests.Session.get and walks the HTTP code path against a fixture; existing native tests stay green. 27 unit tests pass; live native mode against VoltDB 14.2 still emits 44 metric families / 184 series cleanly. * [voltdb] Add hosts list for native multi-server failover Lets the Agent connect to whichever VoltDB cluster member is reachable instead of pinning to a single host. New `hosts` instance option takes a list of `hostname` or `hostname:port` strings; the native Client tries them in order on each (re)connect and surfaces the last error only when every endpoint refuses. Backwards compatible: single-`host:` configs keep working unchanged (they expand to a one-entry endpoint list). `hosts` takes precedence when both are set so users can opt into failover with a single add. Tested live against a local VoltDB 15.3 cluster: - `host: localhost` -> 44 metric families, 184 series (unchanged). - `hosts: [dead.example:21212, localhost:21212]` -> dead endpoint is skipped with a warning, real cluster picks up, active_endpoint correctly reflects 'localhost:21212'. Also tested live HTTP/VMC mode against the local VMC at port 8080: 44 metric families, 208 series, service check OK. Confirms the HTTP client wraps VMC's JSON response into the same shape `_execute_query_raw` expects and the unified code path works for both transports. * [voltdb] Fix CI lint with pinned ruff 0.11.10 isort grouping CI runs ruff 0.11.10 under --config ../pyproject.toml from inside the integration directory. That older ruff is stricter about the isort boundary between third-party and first-party imports than the 0.15+ I had installed locally, so what passed on my machine still failed on the runner with 7 I001 errors across the tests/ tree. Pinned my local ruff to 0.11.10 to match CI exactly, ran `ruff check --fix --config ../pyproject.toml .`, and confirmed the resulting layout is what CI expects. 35 unit tests still pass. Also picks up the license-header fix on the new http_client.py (2020-present -> 2026-present, what ddev validate license-headers --fix expects for a newly added file) and the latest sync of LICENSE-3rdparty.csv. * [voltdb] Re-sync LICENSE-3rdparty.csv to match CI generation * [voltdb] Update integration fixture to new Client(endpoints=) API * [voltdb] Use ca.pem directly as TLS truststore in tests voltdbclient supports pointing ssl_config_file at a PEM truststore without needing a Java keystore properties file. The compose fixtures already ship ca.pem; just reuse it for both host-side integration tests (tests/common.py:TLS_CONFIG_FILE) and the agent-container e2e path (tests/conftest.py:dd_environment). Fixes the FileNotFoundError for client_ssl.properties on the with-tls matrix variants. * [voltdb] Add unit tests for Client and HttpClient code paths Codecov flagged 53 lines without coverage on the patch. Most were in client.py's call_procedure retry-once path, type inference, and close() error swallowing, plus HttpClient's parameter encoding. New tests added (8): - test_client_requires_at_least_one_endpoint - test_client_call_procedure_returns_response - test_client_call_procedure_retries_once_on_stale_connection - test_client_raise_for_status (success + VoltDBError path) - test_client_close_is_idempotent (no-conn + open-then-close) - test_infer_volt_type_distinguishes_bool_int_float_string - test_http_client_serializes_list_params_as_json - test_http_client_raise_for_status Local unit-test coverage: client.py 53% -> 91% (+38pp) http_client.py 89% -> 94% (+5pp) total 88% -> 94% (+6pp) All 43 unit tests pass; ruff check and ruff format --diff --check both green under CI's pinned ruff 0.11.10. * [voltdb] Address @lucia-sb review feedback - client.py / http_client.py: convert `# type:` comments to PEP 484 type annotations on function signatures and locals (per #23667 inline comment on Client.__init__). - spec.yaml: add explicit `default:` keys for `port`, `use_ssl`, `password_hashed`, and `connect_timeout` so the model defaults match what conf.yaml.example documents. - check.py:_fetch_version: look up KEY/VALUE columns by name in the @systemInformation OVERVIEW response so the check tolerates VoltDB versions that reorder or add columns (matches the by-name pattern we already use for @statistics). - tests/test_unit.py:test_http_mode_end_to_end: mock HttpClient directly instead of requests.Session.get; cleaner and doesn't reach into a third-party module. - hatch.toml + tests/common.py: add a `transport` matrix dimension (`native` / `http`) so the existing E2E suite exercises both transports against the same docker fixture. BASE_INSTANCE builds a `url:`-based instance when VOLTDB_TRANSPORT=http and a `host:`-based instance otherwise. - tests/compose/docker-compose.yaml: expose the HTTP/JSON port (VOLTDB_HTTP_PORT) so the new transport dimension can hit it. - tests/conftest.py: use `tls_ca_cert` for HTTP-mode TLS and `ssl_config_file` for native-mode TLS. - tests/utils.py: priming step always uses the native client at the configured client port, regardless of which transport the check under test is exercising. Live-tested both transports against a local VoltDB 15.3 cluster: both report version 15.3.0-SNAPSHOT (the version lookup goes through the by-name path), 44 metric families. * [voltdb] Fix CI failures from the new transport matrix dimension - .github/workflows/test-all.yml: ran 'ddev validate ci --sync' to regenerate the per-target matrix expansion now that voltdb has eight target-envs (version × tls × transport). Without this the validate-ci check fails. - tests/conftest.py:instance_all: always construct a native-shaped instance regardless of VOLTDB_TRANSPORT. Unit tests using the mock_results fixture mock datadog_checks.voltdb.check.Client (the native client), so a url:-based BASE_INSTANCE under the http matrix variant left HttpClient unmocked and the metrics-fixtures test failed. Unit tests are transport-agnostic by design — only the integration tests need to exercise both. - tests/test_integration.py:test_failure_connection_refused: be transport-aware. When url: is the configured transport, rewrite the host portion of the URL (and set a 2s `timeout` for HTTP); otherwise override `host:` and `connect_timeout` as before. The expected port in the service-check tags also depends on which transport is in play. * [voltdb] Set tls_ca_cert on integration instance for http+tls The local integration tests run on the host (not in the agent container) and were hitting SSLCertVerificationError against the self-signed server cert in the with-tls-http matrix variant. The conftest already set tls_ca_cert on the e2e copy of the instance but not on the fixture returned to the in-process tests, so the RequestsWrapper had no truststore to verify against. Mirror what we do for the e2e instance: when TLS_ENABLED and the transport is http, set instance['tls_ca_cert'] = common.TLS_CONFIG_FILE on the test-side instance fixture. Native+tls keeps using ssl_config_file as before. * Update client version to 15.3.0 to be on latest. other sync changes. * sync license. * Update dependency resolution * Fix changelog entry filename to match PR number Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Update dependency resolution * Apply documentation review suggestions to voltdb spec and README Addresses the wording suggestions from the documentation review on #24714: prefer "using" over "via", trim "to use to authenticate", and clarify that selecting the HTTP transport means leaving `host` unset and setting `url`. Regenerates conf.yaml.example from the spec. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Anish Khanzode <akhanzode@voltactivedata.com> Co-authored-by: Anish Khanzode <akhanzode@gmail.com> Co-authored-by: dd-agent-integrations-bot[bot] <dd-agent-integrations-bot[bot]@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> 379948b
1 parent bec852a commit 12db0c5

2 files changed

Lines changed: 2 additions & 2 deletions

File tree

0 commit comments

Comments
 (0)