Repository navigation
[Debugger] Add root timestamp to snapshot payloads - #9532
Merged
Merged
Conversation
The log probes RFC sends the snapshot timestamp both at the root and in debugger.snapshot, with the same value. The logs intake dates the event from the root field, which .NET didn't send.
Contributor
Snapshots difference summaryThe following differences have been observed in committed snapshots. It is meant to help the reviewer. 209 occurrences of : + "timestamp": "ScrubbedValue"
|
dudikeleti
marked this pull request as ready for review
October 27, 2026 14:10
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
More details
The shared finalization path keeps root and nested timestamps identical for Dynamic Instrumentation and Exception Replay. The approval payload changes add only the intended root timestamp.
🤖 Bits Code Review · Commit a1afbaa
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1afbaa117
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This was referenced Oct 27, 2026
tylfin
approved these changes
Oct 27, 2026
dudikeleti
enabled auto-merge (squash)
October 27, 2026 14:54
dudikeleti
added a commit
to DataDog/system-tests
that referenced
this pull request
Oct 27, 2026
… Node.js Released .NET versions before 3.57.0 and Node.js versions before 5.133.0 and 6.22.0 only set debugger.snapshot.timestamp (DEBUG-6442). The fixes are DataDog/dd-trace-dotnet#9532 and DataDog/dd-trace-js#10939. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
BenchmarksBenchmark execution time: 2026-10-27 15:46:26 Comparing candidate commit 4393395 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 72 metrics, 0 unstable metrics, 5 known flaky benchmarks, 121 flaky benchmarks without significant changes.
|
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 of changes
timestampat the root, in epoch milliseconds, with the same value asdebugger.snapshot.timestamp.DebuggerSnapshotCreatorreads the clock once per snapshot and writes that value both indebugger.snapshotand at the root (AddGeneralInfo).timestamp, and a new unit test checks that both fields are present and equal.Reason for change
timestampboth at the root and indebugger.snapshot, in epoch milliseconds with the same value. Java, Python, Ruby and PHP send both. .NET only sentdebugger.snapshot.timestamp.start_timefilter intests/debugger/utils.py) using the roottimestamp. On .NET that filter never applies, soTest_Debugger_InProduct_Enablement_Exception_Replay::test_inproduct_enablement_exception_replay_apm_multiconfigmatches asimple exceptionsnapshot left over from the previous test. In system-tests run 40476449992 (pocdevjob), the payloads inlogs_debugger_inproduct_enablement/interfaces/agent/*__api_v2_debugger.jsonhave the top-level keysdd.span_id, dd.trace_id, ddsource, debugger, debugger.product, debugger.type, logger, message, process_tags, serviceand notimestamp. [debugger] Enable .NET in-product enablement tests from 3.57.0 system-tests#7876 depends on this change.Implementation details
captureSnapshot) and every Exception Replay snapshot is finalized throughFinalizeSnapshot(EndSnapshot, thenAddGeneralInfo).ExceptionReplaySnapshotCreator.EndSnapshotcalls the base method. So one change covers all of them.GetTimestampMs()), so the clock is read once and the two fields always match.debugger.snapshot.timestampkeeps its value, its units and the point where it's taken (EndSnapshot,DateTimeOffset.Now.ToUnixTimeMilliseconds()).longproperty per payload.Test coverage
DebuggerSnapshotCreatorTests.Timestamp_IsWrittenAtRootAndInSnapshotWithSameValuecovers Dynamic Instrumentation with and without a full snapshot, and Exception Replay. It checks that both fields are integers, are equal, and fall within the time of the call.tracer/test/Datadog.Trace.Debugger.IntegrationTests/Approvals/snapshots(209 payloads). Each payload only gains"timestamp": "ScrubbedValue"at the root. TheApproveralready scrubstimestampat every level, so no scrubber change was needed.dotnet test tracer/test/Datadog.Trace.Tests -c Release -f net10.0 --filter "FullyQualifiedName~Datadog.Trace.Tests.Debugger": 880 passed, 2 skipped.-f net48: 844 passed, 29 skipped.tracer\build.cmd BuildAndRunDebuggerIntegrationTests --framework net8.0 --TargetPlatform x64 --DebugType portable --Optimize true --filter "FullyQualifiedName~Datadog.Trace.Debugger.IntegrationTests.ProbesTests": 129 passed, 46 skipped, 0 failed. This exercised 93 of the 95 probe test types whose approvals changed.FullSnapshotWithConditionErrorandNestedRefStructLockLocalTestdidn't run on net8.0.AspNetCore5ExceptionReplay.TestExceptionReplayis skipped on master).Other details
debugger.typeanddebugger.product, which every current payload has, because their tests are skipped (for exampleAspNetCore5ExceptionReplay.TestExceptionReplay), expect no snapshots, or no longer exist. They were stale before this change.masteris on 3.57.0, which isn't released yet. I expect this to make 3.57.0 if it merges before the release is cut. [debugger] Enable .NET in-product enablement tests from 3.57.0 system-tests#7876 enables the multiconfig Exception Replay test from 3.57.0 and needs this fix in thedevbuild first.send.js). It isn't part of this PR.