Skip to content

fix(series): send full attrs until collector confirms series ID (multi-hop fix) - #59

Merged
zzylol merged 1 commit into
mainfrom
45-distributed-sdks---1-collector-distributed-collectors
Mar 19, 2026
Merged

zzylol merged 1 commit into
mainfrom
45-distributed-sdks---1-collector-distributed-collectors

Conversation

@zzylol

@zzylol zzylol commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Root cause: assignSeriesID was setting the local series ID and marking entry.registered=true before any collector response. This caused seriesIdentity to drop dimension attributes even on the first export — including in Agent → Gateway deployments where the gateway never received any attribute mapping.
  • Fix: assignSeriesID now only switches to ID-only mode after Apply() sets entry.registered=true (collector-confirmed). All five sketch annotate* functions guard AttrsClearer behind dp.SeriesID != 0, preventing the aggregator's attrs from being cleared prematurely.
  • Docs: Added docs/otel-distributed-setup.md summarising OTel distributed collector architecture with link references.

What breaks without this fix

In a multi-hop deployment (SDK → Agent → Gateway):

  • Every export sends {series_id: N, attrs: nil} — even the first one
  • The Agent forwards {series_id: N, attrs: nil} to the Gateway
  • The Gateway has no ID → attrs mapping; data points arrive with no dimension labels
  • Agent counting (resource attributes) is unaffected — only data-point labels are lost

Behaviour after fix

Export cycle Before fix After fix
1 (no collector response) series_id=1, attrs=nil series_id=0, attrs=real
2 (no collector response) series_id=1, attrs=nil series_id=0, attrs=real
After Apply() confirms series_id=1, attrs=nil series_id=42, attrs=nil

Test plan

  • TestMultiHop_AttrsUntilCollectorConfirms — drives full three-phase lifecycle (unconfirmed → Apply → confirmed), asserts SeriesID==0 + full attrs on pre-confirmation exports, correct aggregator state transitions after confirmation
  • TestMultiHop_ResourceAttrsAlwaysSent — regression guard that Annotate never touches ResourceMetrics.Resource attributes (service.instance.id, host.name, k8s.pod.name)
  • Run: cd opentelemetry-go/exporters/otlp/otlpmetric/otlpmetricgrpc && go test ./internal/series/... -run TestMultiHop

🤖 Generated with Claude Code

…multi-hop tests

The series ID optimisation was premature: assignSeriesID set the local
ID and marked entry.registered=true before any collector response,
causing seriesIdentity to drop dimension attributes even on the first
export. In a multi-hop deployment (SDK → Agent → Gateway) every
intermediate hop therefore received data points with attrs=nil and an
opaque numeric ID it had no mapping for, silently losing all dimension
labels in the backend.

Fix:
- assignSeriesID now only switches to ID-only mode (sets *seriesID,
  clears *attrs) when entry.registered is already true, i.e. after
  Apply() has been called with a collector-confirmed SeriesAssignment.
  Until that point, seriesID stays 0 so the transform serialises full
  attributes onto the wire.
- All five sketch annotate functions (DDSketch, KLL, CountSketch,
  CountMinSketch, HLL) now guard AttrsClearer behind dp.SeriesID != 0,
  preventing the aggregator's attrs field from being zeroed out before
  confirmation — which would have caused attrs to disappear on the next
  export cycle.

Add two tests in the series package:
- TestMultiHop_AttrsUntilCollectorConfirms: drives the full three-phase
  lifecycle (unconfirmed → Apply → confirmed) and asserts that
  SeriesID==0 + full attrs are forwarded on every pre-confirmation
  export, and that the aggregator state (SeriesIDSink / AttrsClearer)
  transitions correctly only after confirmation.
- TestMultiHop_ResourceAttrsAlwaysSent: regression guard that Annotate
  never touches ResourceMetrics.Resource attributes (service.instance.id,
  host.name, k8s.pod.name), which are the labels used for agent counting
  and Prometheus scrape discovery.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@zzylol zzylol linked an issue Mar 19, 2026 that may be closed by this pull request
@zzylol
zzylol merged commit 014834c into main Mar 19, 2026
@zzylol
zzylol deleted the 45-distributed-sdks---1-collector-distributed-collectors branch March 19, 2026 21:59
SieDeta pushed a commit that referenced this pull request Apr 17, 2026
…ctor-distributed-collectors

fix(series): send full attrs until collector confirms series ID (multi-hop fix)
zzylol added a commit that referenced this pull request Apr 21, 2026
Backend's /metrics endpoint is now populated (ASAPQuery-backend PR
#59), so the scrape job can come back. Point at `backend:9091`
(query server port, --query-port) instead of the stale `:9465`
that was never bound — ingest port 9090 is data-only and does
not expose /metrics.

Live-verified:
  * `asap_ingest_samples_total` at 18.5M through Prometheus
  * Backend target UP alongside controller + gateway
  * Query histograms visible for P99 calculation (§6.3)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

distributed SDKs -> 1 collector, distributed collectors

1 participant