Skip to content

[SDTEST-3777] Gather remaining CI Visibility instrumentation telemetry metrics#274

Merged
ypopovych merged 11 commits into
mainfrom
yehor.popovych/sdtest-3777-remaining-metrics
Jun 5, 2026
Merged

[SDTEST-3777] Gather remaining CI Visibility instrumentation telemetry metrics#274
ypopovych merged 11 commits into
mainfrom
yehor.popovych/sdtest-3777-remaining-metrics

Conversation

@ypopovych

@ypopovych ypopovych commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Event lifecycle metrics: events.created / events.finished emitted at test hooks (session/module/suite/test start and finish) via new TelemetryEventsFeature, matching dd-trace-go timing
  • Git SHA discrepancy metrics: git.commit_sha_match / git.commit_sha_discrepancy — detected in Environment.Git at init time, carried through all builder methods, and emitted via TestSession.emitGitShaCheck(to:) shared by both auto and manual API paths
  • Git command metrics: git.command / git.command_errors with real exit codes from Spawn.RunError.exitCode, unified into a single recordedGitCommand helper in GitUploader
  • Code coverage metrics: code_coverage.is_empty, code_coverage.files emitted after coverage collection
  • ITR skip metrics: itr.skipped_tests / itr.unskippable_tests / itr.forced_run_tests
  • Settings response metrics: endpoint.response counts for settings calls, gated on telemetry presence and recorded before any throws
  • App lifecycle telemetry: app-started sent immediately at init (direct HTTP), heartbeat via DispatchSourceTimer, app-closing batched before final flush
  • impacted_tests_enabled wired from settings JSON → TracerSettings → telemetry config
  • NTPClock crash fix: New FallbackClock wrapper absorbs sync failures internally, replacing the clock var; sync now runs before API creation so captures are always safe
  • SessionConfig refactor: holds env: Environment and config: Config directly, removing duplicate properties; TestSession/TestModule/TestSuite protocols expose configuration: SessionConfig

Test plan

  • xcodebuild test -scheme DatadogSDKTesting passes all unit tests
  • TelemetryTests cover metric emission, app lifecycle, and observer wiring
  • LibraryConfigurationErrorsTests covers settings decode including impacted_tests_enabled
  • FileWriterTests verifies eventEnqueued() fires before encode
  • SessionManagerTests / SwiftTestingTraitTests compile and pass with new SessionConfig API

🤖 Generated with Claude Code

ypopovych and others added 11 commits June 3, 2026 18:35
…y metrics

- event_created/event_finished/test_session via TelemetryEventsFeature (fires at
  start hooks matching dd-trace-go; reads CI provider from SessionConfig.env)
- events.enqueuedForSerialization via PayloadObserver.eventEnqueued() per write
- events.manualApiEvents in @objc manual-API entry points
- git.command/command_ms/command_errors via GitUploader.recordedGitCommand helper
  (real exit codes via public Spawn.RunError.exitCode; 128+signal convention)
- git_requests.objects_pack_files, settings_response (with impacted_tests_enabled
  from JSON), response counts for TIA/known-tests/test-management
- itr.skipped/unskippable/forced_run in TestImpactAnalysis hooks
- code_coverage.started/finished/is_empty/errors/files through the coverage stack
- error_type granularity: RequestObserver carries transportError so URLError.timedOut
  maps to .timeout instead of .network
- App-lifecycle protocol: app-started (direct, with SDK config snapshot), heartbeat
  timer, app-closing batched before final flush
- SessionConfig now holds env: Environment and config: Config; removed duplicate
  platform/service/metrics properties; TestSession/Module/Suite protocols gained
  var configuration so features access CI/git without touching DDTestMonitor

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…etrics

Detects commit-SHA and repository-URL discrepancies between the three git
info providers (ci_provider, local_git, user_supplied) while assembling
Environment.git. Discrepancies are stored directly on the Git struct as
[ShaDiscrepancy] so callers access env.git.discrepancies / shaMatched
without a separate wrapper type.

Emitted once per session start via TelemetryEventsFeature (auto path) and
DDSession.start() (manual path).

Also moves FileWriter.eventEnqueued() before encoder.encode() so it fires
at the point an event enters the write queue rather than inside the file
orchestrator closure.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Extract emitGitShaCheck(to:) into a TestSession extension so both the
auto-instrumented path (TelemetryEventsFeature.testSessionWillStart) and
the manual API path (DDSession.start) share a single call site.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
The configuration accessor on modules and suites just walked up the
parent chain (session.configuration / module.configuration), duplicating
a value already reachable via the existing session/module accessors.
Drop the per-type declarations and the TestModule/TestSuite protocol
requirements in favor of default implementations, mirroring the existing
`session` convenience extensions. Only TestSession keeps configuration as
a real (stored) requirement. Applied the same cleanup to the test mocks.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
- Telemetry: make telemetryExporter optional so the new test-only init
  (testMetricExporter:resource:exportInterval:) can skip lifecycle
  (app-started, heartbeat, app-closing) without needing a real
  TelemetryExporter or TelemetryApi
- TelemetryTests: use new test init; add missing transportError: nil to
  requestFinished calls
- SwiftTestingTraitTests: update SessionConfig construction to the new
  (env:config:) signature, removing the removed platform/service/metrics
  parameters

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
PayloadObserver gained eventEnqueued() when the per-event enqueue hook
was added; update the test spy to conform.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Add SyncingClock wrapper: wraps any Clock and replaces the inner clock
with DateClock on sync failure (via Synced<any Clock>). `now` always
works — no crash regardless of sync state. Removes all DDTestMonitor.clock
= DateClock() replacement sites.

DDTestMonitor.clock is now always a SyncingClock. The convenience init in
DDTracer calls sync() before creating the API service so the date provider
is ready when app-started fires.

Remove the test-only init from production Telemetry.swift. Move it to
TelemetryTests.swift as a convenience init extension that delegates to
the production init using a NoopTelemetryApi stub.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…nabled to mock response

The validSettingsBody helper was missing the impacted_tests_enabled field
added to SettingsResponse; JSON decoding now requires it, so the test was
failing with a keyNotFound error wrapped as a communication error.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
TestSessionManagerObserver and TestModuleManagerObserver took the config
alongside the session/module, but the model now exposes it directly via
session.configuration / module.configuration. Drop the `with config:`
parameter from all six methods and read it off the model inside the only
real conformer (SessionAndModuleObserver).

This also removes the now-unused stored `config` from DDModule's
Stateless/StatefulManager and their initializers, since they only held it
to forward to the observer.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The manager exposed session, config, and a sessionAndConfig tuple, but
every consumer split the tuple and stored config separately even though
the session now carries it via session.configuration. Reduce the protocol
to a single `session` getter and drop config/sessionAndConfig plus the
derivation extension.

Removes the now-redundant duplicate config storage that rode alongside
the model in DDXCTestObserver.ModuleContext, SwiftTestingContext's
ModuleContext and the suite factory, and SwiftTestingSuiteContext (whose
configuration is now derived from suite.configuration). Callers read
config off the model instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@ypopovych
ypopovych requested review from a team as code owners June 5, 2026 14:08
@ypopovych
ypopovych merged commit ad6793e into main Jun 5, 2026
11 checks passed
@ypopovych
ypopovych deleted the yehor.popovych/sdtest-3777-remaining-metrics branch June 5, 2026 14:23
ypopovych added a commit that referenced this pull request Jun 5, 2026
#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) <[email protected]>
ypopovych added a commit that referenced this pull request Jun 8, 2026
* Replace OTel metrics SDK with a custom telemetry collector

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) <[email protected]>

* Make telemetry flush interval and distribution buffer size configurable

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) <[email protected]>

* Key telemetry metrics by a tag Set instead of a sorted array

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) <[email protected]>

* Guard MetricStore state with Synced instead of a bare UnfairLock

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) <[email protected]>

* Raise default distribution buffer cap to 65536

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) <[email protected]>

* Stamp telemetry metric points with the SDK clock, not Date()

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) <[email protected]>

* Remove unused telemetry namespace properties from Resource

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) <[email protected]>

* Merge duplicated metric types in the telemetry tree

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) <[email protected]>

* Add missing impacted_tests_enabled to mock backend settings response

#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) <[email protected]>

* Add telemetry integration coverage; flush telemetry synchronously at 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) <[email protected]>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
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