[Bug]: client telemetry and tracing — sampling suppression, SDK correctness gaps, unusable async API, stale MEP

[xiaofan-luan]

Umbrella issue for an audit of the client telemetry (ClientTelemetryService) and client-side tracing paths. Five problems, one PR.

---

1. client_request_id silently disables trace sampling (the actual bug)

A request carrying the client_request_id (or legacy client-request-id) header is never sampled, regardless of trace.sampleFraction, and the suppression propagates to every downstream component — the whole call tree is missing from Jaeger/OTLP.

It is silent because the header still does its main job: the trace ID reaches the logs. You keep log correlation and lose tracing with no signal.

Cause. clientRequestIDPropagator.Extract carries the client-supplied ID in a synthesized remote parent span context with no sampled flag (pkg/tracer/client_request_id_propagator.go:52-56):

return trace.ContextWithRemoteSpanContext(ctx, trace.NewSpanContext(trace.SpanContextConfig{
    TraceID: traceID,
    SpanID:  spanID,
    Remote:  true,
    // TraceFlags defaults to 0x00 => not sampled
}))

SetTracerProvider uses ParentBased(TraceIDRatioBased(fraction)) (pkg/tracer/tracer.go:68-76), and OTel's ParentBased defaults remoteParentNotSampled to NeverSample() (sdk@v1.43.0 trace/sampling.go:200). The root sampler — the only one that consults sampleFraction — is reached only when there is no parent at all. Synthesizing a parent takes the decision away from the operator and hardcodes it to drop.

The underlying design error: a client-supplied correlation ID was placed in the parent span context, a slot that in OTel also encodes an upstream sampling decision.

Repro:

1. Start Milvus with trace.exporter=jaeger and trace.sampleFraction=1.0
2. Search with no trace headers                    -> trace appears
3. Same search with client_request_id set to 32 hex
   (pymilvus: CallContext(client_request_id=...))  -> no trace, at any component
   The trace ID is still in the logs, which is why this goes unnoticed.

Blast radius: users who explicitly set a request ID — i.e. people actively debugging, exactly when traces matter most. The common support flow ("send us a request id") currently disables tracing for the request being investigated.

1b. Propagator ordering makes the guard dead code

propagation.NewCompositeTextMapPropagator(
    clientRequestIDPropagator{},   // runs first
    otel.GetTextMapPropagator(),   // TraceContext + Baggage, runs second
)

clientRequestIDPropagator.Extract opens with if trace.SpanContextFromContext(ctx).IsValid() { return ctx }, a guard meant to yield to a real traceparent. Running first, nothing has been extracted yet, so it can never fire. The two only compose correctly because TraceContext runs later and overwrites the synthesized context. Reordering the composite would silently flip behavior.

---

2. Go SDK telemetry correctness gaps

The Go SDK is currently the only SDK implementing the client half. The protocol-critical parts (config_hash algorithm, last_command_timestamp watermark, reply retry-on-failed-heartbeat, reservoir P99) are correct; these are the gaps around them.

• Doomed heartbeat retried forever. Against a server predating ClientTelemetryService, the heartbeat fires every 30s for the lifetime of the client with no backoff and no self-disable. The error is dropped despite a comment claiming it is logged:

  if err != nil {
      // Log error but continue - telemetry is best-effort
      return   // nothing is actually logged
  }

  connectInternal already handles Unimplemented this way; the heartbeat path missed it. The client-wide retry interceptor also applies, so a heartbeat against an Unavailable server costs six RPCs with backoff instead of one.
• max_latency_ms collected but never sent. common.Metrics has the field and the collector computes it, but every proto conversion site omitted it, so the server could never display it.
• client_id not stable across restarts. A fresh uuid.New() per process, so server-side history fragments on every restart and client:<id> scoped commands cannot target a long-lived client.
• A test-only fork that had already diverged. processCommands / calculateConfigHash / collectProtoMetrics / getPendingReplies were reachable only from tests. The test-only calculateConfigHash hashed ID:Type while production hashes ID + Type + payload — and config_hash must match the server byte-for-byte or configs are re-pushed on every heartbeat. The tests were validating an algorithm that never runs.
• No request-ID propagation. pymilvus exposes CallContext(client_request_id=...); the Go SDK had no equivalent, so there was no way to correlate a Go client call with server-side logs.

---

3. Async telemetry API cannot be consumed programmatically

GET /_telemetry/clients/{id}/config and .../history push a command and return {"command_id": ..., "status": "pending"}. The reply lands in a StoredCommandReply that no endpoint exposes — and it is smuggled through ClientInfo.Reserved["command_replies"] as a JSON string. TelemetryManager.GetClientCommandReplies() exists but has no production caller.

The only way to read a result was to list every client and scan the array by hand. Every consumer that is not the WebUI — a script, a CLI, an agent — has to reimplement push → poll-all-clients → match → time out.

---

4. The MEP does not match the implementation

docs/design-docs/design_docs/20260131-client_side_telemetry.md was written before the feature landed and never updated. Anyone implementing the client half for another SDK from it would produce something that does not interoperate.

Doc says	Reality
set_sampling_rate / enable_collections / update_config	None exist. Real set: push_config, collection_metrics, show_errors, show_latency_history, get_config
TargetScope = "*" / "client_id:xxx" / "db:name"	global / client:<id> / database:<db>
REST at /api/v1/telemetry/*	/_telemetry/* on the internal HTTP port
RPCs on service RootCoord	Own service ClientTelemetryService
ClientHeartbeatResponse.NewConfigHash	No such field
"Persistent commands re-sent until deleted"	Suppressed whenever config_hash matches
"Disabled by default"	DefaultTelemetryConfig() has Enabled: true
CommandHandler returns string	Returns *CommandReply

Also missing entirely: the exact config_hash algorithm, the watermark's exactly-once consequence, reply-as-GC semantics, the ttl_seconds: 0 leak, persistent-config JSON merge, the hardcoded server limits, and that there are no milvus.yaml keys for any of it.

---

5. Ecosystem gap (context, not fixed here)

Client-side telemetry exists only in the Go SDK. pymilvus, Java, Node.js and Rust do not implement it — so the feature currently covers none of the Python user base. Conversely, Node.js has full W3C traceparent and pymilvus has client-request-id, while the Go SDK had neither until this PR.

Additional server-side weaknesses found but not addressed here: no milvus.yaml/paramtable keys, Basic Auth with no RBAC, non-persistent commands live only on the RootCoord that received the push (lost on restart, not shared across replicas), ttl_seconds: 0 commands leak forever if a client never replies, and PushCommand does not validate command_type.

[xiaofan-luan]

Two updates to keep this issue accurate.

Label scope: considered, dropped. #51967's commit for the client-scope persistence rule says durable per-workload configuration is "tracked separately in #51963". It was prototyped (milvus-io/milvus-proto#646 + #51969) and both are now closed. Reasoning: push_config exposes exactly three knobs — enabled, heartbeat_interval_ms, sampling_rate — all of them about telemetry itself. Per-workload targeting of those three does not justify a permanent proto field plus a client-side opt-in that applications must adopt before anything matches. database: scope covers most of the need. Worth revisiting only if push_config grows knobs that matter more.

Newly found, not fixed: one-time commands to multi-client scopes have no delivery guarantee.

cleanupRepliedCommands deletes a non-persistent command as soon as any client replies:

for _, id := range commandIDs {
    m.commandStore.DeleteNonPersistentCommand(id)
}

There is no per-client tracking of who has received or answered it. So a one-time command scoped to global or database:<db> is delivered only to clients that heartbeat before the first reply comes back, then disappears. In the common case the window is wide enough that everyone gets it — every client heartbeats within one interval, and the first reply arrives an interval later — but it is a race, not a guarantee. A client that is slow, paused, or restarting misses it silently, and there is no way to see who received it.

This predates and is independent of everything in #51967.

Correction: the "ttl_seconds: 0 commands leak forever" item in the issue body is fixed by #51967, which gives unspecified-TTL commands a default of ten heartbeat cycles.