Skip to content

feat: add client telemetry support - #585

Merged
sre-ci-robot merged 13 commits into
milvus-io:masterfrom
xiaofan-luan:feat/client-telemetry
Aug 25, 2026
Merged

sre-ci-robot merged 13 commits into
milvus-io:masterfrom
xiaofan-luan:feat/client-telemetry

Conversation

@xiaofan-luan

@xiaofan-luan xiaofan-luan commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • implement the shared client telemetry heartbeat and command protocol in the C++ SDK
  • report one metric per public logical Search, Query/Get, HybridSearch, RunAnalyzer, Insert, Delete, and Upsert call
  • preserve telemetry state across reconnects, database switches, and global-cluster topology failover within the same logical endpoint/user/config scope
  • make heartbeat Stop/restart and last-owner destruction safe, including destruction from the heartbeat callback thread
  • keep the command heartbeat active across dynamic disable and Stop/Start reconnects so ACK and re-enable can flow, while an initial user opt-out starts no worker
  • retain a bounded one-hour default history, aggregate P99 from retained samples, and skip empty high-frequency history entries
  • align deterministic sampling, collection filtering, strict atomic command handling, RFC3339 validation, reply acknowledgement, unsupported backoff, request IDs, unset-database reporting, and command cursor idempotency with the Go SDK
  • expose retryable CLIENT_BUSY lifecycle failures and add focused unit plus opt-in end-to-end coverage

Related work

Verification

  • full C++ unit test suite: 842/842 passed
  • focused telemetry and global/lifecycle suites passed
  • end-to-end test target builds successfully
  • LLVM 17 clang-format dry run and git diff --check

Compatibility note

ConnectParam gains telemetry configuration state. Consumers should rebuild against this SDK revision rather than mixing binaries compiled against the previous object layout.

@mergify

mergify Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f69630f982

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/impl/ClientTelemetry.cpp Outdated
Comment thread src/impl/ClientTelemetry.cpp Outdated
Comment thread src/impl/ClientTelemetry.cpp Outdated
Comment thread src/include/milvus/MilvusClientV2.h
Comment thread src/impl/utils/ConnectionHandler.cpp Outdated
Comment thread src/include/milvus/ClientTelemetry.h Outdated
Comment thread test/CMakeLists.txt
Comment thread src/impl/ClientTelemetry.cpp Outdated
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Sampling kept a counter modulo 10000 and recorded an operation while the
remainder was under rate*10000, so the sampled operations formed one
contiguous run per cycle. A cycle is ten thousand operations while a
heartbeat window is tens or hundreds, so every window came out wholly
sampled or wholly dropped: at 3 QPS a rate of 0.25 gave fourteen minutes
of full metrics followed by forty-one reporting nothing from a client
that never stopped working. The long-run ratio was right; the ratio
inside a window, which is the only unit the telemetry API reports, never
was.

Accumulate the rate instead and sample on the operation that carries the
accumulator across a whole unit: 0.25 samples every fourth operation, and
any stretch of operations holds the configured ratio. A rate too small to
represent now still samples rarely rather than never -- the old threshold
truncated anything below 1e-4 to zero and then sampled nothing, so a
configured rate silently meant "off".

Matches the Go client, milvus-io/milvus#52615.

Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
The heartbeat interval is also the metrics window: each heartbeat carries
the operations since the last one, and the coordinator answers a
telemetry query from the window before the newest, so a caller reads
data between one and two intervals old. At the previous default of 30
seconds that is up to a minute behind, which reads as a stalled client
rather than as a lagging view.

Ten seconds puts the worst case at twenty. The cost is the coordinator
heartbeat rate, which scales with the number of connected clients rather
than with traffic; a deployment with enough clients for that to matter
can raise the interval, per client or fleet-wide, with a persistent
push_config.

Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
@xiaofan-luan
xiaofan-luan force-pushed the feat/client-telemetry branch from 722e13c to 8eec85c Compare August 24, 2026 09:42
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.98362% with 198 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.28%. Comparing base (a0592aa) to head (dcee0f4).
⚠️ Report is 149 commits behind head on master.

Files with missing lines Patch % Lines
src/impl/ClientTelemetry.cpp 79.70% 164 Missing ⚠️
src/impl/utils/ConnectionHandler.cpp 87.85% 13 Missing ⚠️
src/impl/MilvusConnection.cpp 86.00% 7 Missing ⚠️
src/impl/MilvusClientImpl.cpp 82.85% 6 Missing ⚠️
src/impl/types/ConnectParam.cpp 62.50% 3 Missing ⚠️
src/include/milvus/MilvusClient.h 0.00% 2 Missing ⚠️
src/include/milvus/MilvusClientV2.h 0.00% 2 Missing ⚠️
src/impl/utils/TelemetryUtils.h 94.44% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #585       +/-   ##
===========================================
+ Coverage   53.47%   87.28%   +33.80%     
===========================================
  Files          52      379      +327     
  Lines        4432    15796    +11364     
  Branches        0     1715     +1715     
===========================================
+ Hits         2370    13787    +11417     
+ Misses       2062     2009       -53     
Files with missing lines Coverage Δ
src/impl/ClientRequestContext.cpp 100.00% <100.00%> (ø)
src/impl/MilvusClientImpl.h 100.00% <ø> (ø)
src/impl/MilvusClientV2Impl.cpp 80.12% <100.00%> (ø)
src/impl/MilvusClientV2Impl.h 100.00% <ø> (ø)
src/impl/MilvusConnection.h 95.45% <100.00%> (-1.22%) ⬇️
src/impl/utils/ConnectionHandler.h 90.16% <ø> (ø)
src/include/milvus/types/ConnectParam.h 100.00% <ø> (ø)
src/impl/utils/TelemetryUtils.h 94.44% <94.44%> (ø)
src/include/milvus/MilvusClient.h 33.33% <0.00%> (ø)
src/include/milvus/MilvusClientV2.h 33.33% <0.00%> (ø)
... and 5 more

... and 381 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Comment thread src/impl/utils/ConnectionHandler.cpp Outdated
Comment thread src/impl/MilvusConnection.h
@yhmo
yhmo requested a balanced review from Copilot August 24, 2026 11:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread src/impl/ClientTelemetry.cpp
Comment thread src/impl/MilvusClientImpl.cpp Outdated
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
@mergify mergify Bot added the ci-passed label Aug 24, 2026
@yhmo

yhmo commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

/lgtm
/approve

@sre-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xiaofan-luan, yhmo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot
sre-ci-robot merged commit 776c16d into milvus-io:master Aug 25, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants