[SDTEST-3777] Gather remaining CI Visibility instrumentation telemetry metrics#274
Merged
Merged
Conversation
…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]>
anmarchenko
approved these changes
Jun 5, 2026
3 tasks
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]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
events.created/events.finishedemitted at test hooks (session/module/suite/test start and finish) via newTelemetryEventsFeature, matching dd-trace-go timinggit.commit_sha_match/git.commit_sha_discrepancy— detected inEnvironment.Gitat init time, carried through all builder methods, and emitted viaTestSession.emitGitShaCheck(to:)shared by both auto and manual API pathsgit.command/git.command_errorswith real exit codes fromSpawn.RunError.exitCode, unified into a singlerecordedGitCommandhelper inGitUploadercode_coverage.is_empty,code_coverage.filesemitted after coverage collectionitr.skipped_tests/itr.unskippable_tests/itr.forced_run_testsendpoint.responsecounts for settings calls, gated on telemetry presence and recorded before any throwsapp-startedsent immediately at init (direct HTTP),heartbeatviaDispatchSourceTimer,app-closingbatched before final flushimpacted_tests_enabledwired from settings JSON →TracerSettings→ telemetry configFallbackClockwrapper absorbs sync failures internally, replacing the clock var; sync now runs before API creation so captures are always safeenv: Environmentandconfig: Configdirectly, removing duplicate properties;TestSession/TestModule/TestSuiteprotocols exposeconfiguration: SessionConfigTest plan
xcodebuild test -scheme DatadogSDKTestingpasses all unit testsTelemetryTestscover metric emission, app lifecycle, and observer wiringLibraryConfigurationErrorsTestscovers settings decode includingimpacted_tests_enabledFileWriterTestsverifieseventEnqueued()fires before encodeSessionManagerTests/SwiftTestingTraitTestscompile and pass with newSessionConfigAPI🤖 Generated with Claude Code