Skip to content

Replace OTel metrics SDK with a custom telemetry collector#275

Merged
ypopovych merged 10 commits into
mainfrom
yehor.popovych/custom-telemetry-distributions
Jun 8, 2026
Merged

Replace OTel metrics SDK with a custom telemetry collector#275
ypopovych merged 10 commits into
mainfrom
yehor.popovych/custom-telemetry-distributions

Conversation

@ypopovych

@ypopovych ypopovych commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Replaces the OpenTelemetry metrics SDK behind CI Visibility instrumentation telemetry with a small, purpose-built collector.

The DD telemetry intake wants distributions as raw sample arrays (points: [Double]) and computes the summary server-side — exactly what dd-trace-go sends. The OTel Histogram path bucketed samples and then reconstructed approximate values from bucket midpoints on export: lossy (distorts percentiles) and complex (separate explicit/exponential code paths). Counters/gauges were trivial sums riding the same heavyweight machinery.

This swaps that for a lock-protected MetricStore:

  • Counters accumulate a per-interval delta (backend sums deltas).
  • Distributions keep every raw sample (backend computes the summary).
  • Each drain takes-and-resets the buffers (delta semantics) and writes to the durable TelemetryExporter.

The whole OTel metrics layer — meter provider, views, periodic reader, and the TelemetryMetricExporter bridge — is removed (−1018 lines).

Robustness refinements

  • Short flush interval (default 10s) shrinks the window of in-memory metrics a crash can lose. export(item:) persists to disk immediately and upload happens later on the exporter's own schedule, so this adds no network traffic.
  • Force-flush on a full distribution buffer so a burst is persisted rather than dropped (cf. dd-trace-go, which drops on a full ring buffer).

Configurable (env-overridable, mirroring DD_TELEMETRY_HEARTBEAT_INTERVAL)

  • DD_TELEMETRY_FLUSH_INTERVAL — metric drain cadence (seconds, clamped 1…3600, default 10)
  • DD_TELEMETRY_DISTRIBUTION_BUFFER_SIZE — force-flush threshold (samples, clamped ≥ 1, default 65535)

Tag keying

Metrics are keyed by (name, Set<String> tags) — same name + different tags = separate series, matching dd-trace-go (metricKey includes tags). Using a Set makes order-independence intrinsic and dedups; the wire array is sorted once per series at emit time.

Preserved from #274

The app-lifecycle protocol (app-started direct send, app-heartbeat timer, app-closing on shutdown) is kept and now rides the custom collector. Heartbeat keeps its own DD_TELEMETRY_HEARTBEAT_INTERVAL, independent of the metric flush cadence.

Notes

  • The metric tree and tag enums are unchanged; only the Counter/Distribution/Factory internals now target the store.
  • TelemetryExporter gains a small TelemetryPayloadExporter sink protocol so the collector is unit-testable.
  • Tests rewritten against an in-memory payload sink; ARCHITECTURE.md updated.

Test plan

  • Framework build succeeds
  • Test build succeeds
  • TelemetryTests, ConfigTests, DDTracerTests, TelemetryExporterTests pass

🤖 Generated with Claude Code

ypopovych and others added 3 commits June 5, 2026 15:40
Telemetry distributions are sent to the DD intake as raw sample arrays so the
backend computes the summary. The OTel-histogram path bucketed samples and then
reconstructed approximate values from bucket midpoints on export — lossy
(distorts percentiles) and complex (separate explicit/exponential code paths).

Drop the OpenTelemetry metrics SDK (meter provider, views, periodic reader, the
TelemetryMetricExporter bridge) in favor of a small lock-protected MetricStore:
counters accumulate a per-interval delta, distributions keep raw samples, and
each drain takes-and-resets the buffers (delta semantics). The store is drained
to the durable TelemetryExporter via a new TelemetryPayloadExporter sink
protocol.

Two robustness refinements:
- A short flush interval (10s) shrinks the window of in-memory metrics a crash
  could lose — export() persists to disk immediately and upload happens later on
  the exporter's own schedule, so this adds no network traffic.
- When buffered distribution samples reach a cap, record() force-flushes so a
  burst is persisted rather than dropped (dd-trace-go drops on a full buffer).

The metric tree and tag enums are unchanged; only the Counter/Distribution/
Factory handle internals now target the store. Tests rewritten against an
in-memory payload sink; ARCHITECTURE.md updated.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire the two collector tunables to Config so they can be overridden by env vars,
mirroring DD_TELEMETRY_HEARTBEAT_INTERVAL:
- DD_TELEMETRY_FLUSH_INTERVAL (seconds, clamped 1...3600, default 10)
- DD_TELEMETRY_DISTRIBUTION_BUFFER_SIZE (samples, clamped >= 1, default 2048)

DDTracer threads them from Config into Telemetry; the hardcoded defaults remain
as the fallback for direct construction.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Set equality/hashing is intrinsically order-independent, so the metric key
no longer relies on remembering to sort tags at record time, and duplicate
"key:value" entries collapse. The wire array is sorted once per series at emit
time for deterministic output. Matches dd-trace-go semantics (same name +
different tags = separate series).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ypopovych
ypopovych requested review from a team as code owners June 5, 2026 14:55
ypopovych and others added 3 commits June 5, 2026 16:01
Wraps the counters/distributions/sample-count in a single Synced<State>,
matching the locking pattern used elsewhere in the SDK (e.g. DDModule) rather
than hand-rolling an UnfairLock. Behavior is unchanged: take-and-reset on flush
via `defer { s = State() }`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The cap is a global safety backstop for bursts; the 10s timer does the normal
draining. Samples are 8-byte Doubles, so a generous cap is cheap (65536 ≈ 512 KB)
and keeps force-flush from triggering under normal CI telemetry volume.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MetricStore now takes the NTP-synced SDK clock (DDTestMonitor.clock) and uses
clock.now for count-series timestamps instead of a raw Date(), matching the
rest of the SDK's time handling. Tests pass a DateClock().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@datadog-datadog-prod-us1

This comment has been minimized.

ypopovych and others added 4 commits June 5, 2026 16:36
telemetryMetricNamespace / telemetryDistributionNamespace were only read by the
deleted OTel TelemetryMetricExporter bridge. The custom collector stamps the
.civisibility namespace directly on each payload, so these Resource accessors
have no remaining readers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Most metrics differed only in name, not shape: an untagged counter, an untagged
distribution, or a counter tagged solely by error_type / event_type / endpoint.
Replace the ~30 near-identical leaf structs with six reusable handle types
(NoTagCounter, NoTagDistribution, ErrorTypeCounter, EventTypeCounter,
EndpointCounter, EndpointDistribution) plus matching Factory helpers.

Metrics with a unique tag signature (events.created/finished, session.started,
git.command*, settings_response, requests_errors, code_coverage library
counter) keep their own struct. Call sites and wire output are unchanged;
TelemetryMetrics.swift drops from 682 to 513 lines.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#274 added a required `impactedTestsEnabled` field to the SDK's SettingsResponse
model but didn't update the integration-test mock backend, so the settings
response it returned no longer decoded — failing every integration test that
boots the SDK against the mock.

Emit `impacted_tests_enabled` from MockBackend.buildSettingsResponse (configurable
via Settings.impactedTestsEnabled, default false). Matches the field set the SDK
decoder requires, as covered by LibraryConfigurationServiceThrowTests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…shutdown

Verifying telemetry end-to-end surfaced a real bug: Telemetry.shutdown() drained
the in-memory store to the exporter and enqueued app-closing, but only called
exporter.shutdown() (stop the worker) — never exporter.flush(). Since flush() is
what synchronously uploads, the final batch (self-metrics + app-closing) sat on
disk unsent at process exit; only the direct app-started POST reliably reached
the backend. Add exporter.flush() before shutdown so the last batch is uploaded.

Tooling to assert telemetry reaches the backend:
- MockBackend.parseTelemetry decodes the captured /api/v2/apmtelemetry bodies
  (both the direct single-payload form and the message-batch envelope) into typed
  TelemetrySeries + observed event types, exposed via Requests.telemetry* accessors.
- MockBackendTelemetryParsingTests unit-tests the parser against the wire format.
- A telemetryReported integration test asserts app-started/app-closing events and
  generate-metrics (test_session) round-trip to the backend over a basic run.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ypopovych
ypopovych merged commit 083936c into main Jun 8, 2026
11 checks passed
@ypopovych
ypopovych deleted the yehor.popovych/custom-telemetry-distributions branch June 8, 2026 12:55
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.

2 participants