Skip to content

feat: add client telemetry support - #602

Merged
shanghaikid merged 1 commit into
milvus-io:mainfrom
xiaofan-luan:feat/client-telemetry
Aug 26, 2026
Merged

shanghaikid merged 1 commit into
milvus-io:mainfrom
xiaofan-luan:feat/client-telemetry

Conversation

@xiaofan-luan

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

Copy link
Copy Markdown
Contributor

Summary

  • add a client telemetry manager with heartbeat commands, metrics snapshots, and recent-error reporting
  • report one outcome per public logical operation across validation, retry, and result processing
  • align strict atomic commands, deterministic sampling, collection filtering, RFC3339/request-ID validation, acknowledgement, and unsupported backoff with the Go SDK
  • preserve documented arbitrary request IDs on the legacy wire header while requiring canonical lowercase trace IDs only for telemetry correlation
  • validate a complete candidate lifecycle before failover, preserve the old client on failure, and discard stale endpoint heartbeat responses by epoch
  • keep equal-timestamp command IDs until the timestamp cursor advances so repeated delivery stays idempotent
  • keep the command heartbeat alive after a server-side dynamic disable so its ACK and a later re-enable can flow, while an initial user opt-out starts no loop
  • use a 10-second default heartbeat interval and add focused unit plus opt-in end-to-end coverage

Related work

Verification

  • non-server unit suites: 631 passed
  • telemetry focused suite: 18 passed
  • TypeScript typecheck, Prettier, and git diff --check

@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: 946b7d5e6f

ℹ️ 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 milvus/utils/Function.ts
Comment on lines +410 to +413
export const isValidClientRequestId = (value: unknown): value is string =>
typeof value === 'string' &&
/^[0-9a-f]{32}$/.test(value) &&
value !== '00000000000000000000000000000000';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid silently dropping documented request IDs

When callers use the currently documented IDs such as trace-${Date.now()}, insert-trace-123, or trace-id-123 (docs/src/content/docs/advanced/advanced-features.mdx:156-167,178-207), this new predicate rejects them and executeCall sends no client-request-id metadata. Upgrading therefore silently disables server-side request correlation for existing applications across every RPC, not just telemetry; preserve the prior metadata behavior or provide an explicit compatibility/migration path rather than dropping these values.

Useful? React with 👍 / 👎.

Comment on lines +363 to +365
for (const [id, timestamp] of this.executedCommands) {
if (timestamp <= previousTimestamp) {
this.executedCommands.delete(id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Retain command IDs at the current timestamp

When the same command is delivered in three batches with an unchanged create_time, the second delivery is correctly deduplicated, but this cleanup then deletes its ID because its timestamp equals previousTimestamp; the third delivery executes the handler again. This affects persistent or repeatedly delivered commands and is especially unsafe for custom non-idempotent handlers exposed through registerCommandHandler; keep IDs whose timestamp is still equal to the latest command timestamp.

Useful? React with 👍 / 👎.

Signed-off-by: xiaofanluan <xiaofan.luan@zilliz.com>
@xiaofan-luan
xiaofan-luan force-pushed the feat/client-telemetry branch from d117c5b to 60bfc77 Compare August 25, 2026 19:19
@sre-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

The full list of commands accepted by this bot can be found 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

@shanghaikid
shanghaikid merged commit 7c427fd into milvus-io:main Aug 26, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants