Repository navigation
chore(telemetry): discard the old worker and exporter subscription on MicroVM refresh - #20956
Draft
litianningdatadog wants to merge 2 commits into
Conversation
litianningdatadog
added
changelog/no-changelog
This was referenced Oct 13, 2026
Dependency direction analysis
|
Codeowners resolved asResolved from the full PR diff against |
Circular import analysis
|
Contributor
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🔗 Commit SHA: 6dd7ba4 | Docs | View more details | Give us feedback! |
BenchmarksBenchmark execution time: 2026-10-15 22:29:25 Comparing candidate commit 6dd7ba4 in PR branch Found 0 performance improvements and 3 performance regressions! Performance is the same for 359 metrics, 10 unstable metrics, 4 known flaky benchmarks, 4 flaky benchmarks without significant changes.
|
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
from
October 13, 2026 19:10
62fafe4 to
810c6a1
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
from
October 13, 2026 19:10
6cb67c1 to
359e963
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
from
October 13, 2026 19:25
810c6a1 to
df15134
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
from
October 13, 2026 19:25
359e963 to
787a216
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
from
October 14, 2026 17:35
df15134 to
ea2e306
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
3 times, most recently
from
October 14, 2026 19:56
6087baf to
b16f11d
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
from
October 14, 2026 19:56
ea2e306 to
0549be3
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
from
October 14, 2026 20:27
b16f11d to
7bdf601
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
2 times, most recently
from
October 15, 2026 13:30
b2aaf01 to
5e34037
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
from
October 15, 2026 13:30
7bdf601 to
4f87d3a
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
from
October 15, 2026 14:04
5e34037 to
c00bdc9
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
2 times, most recently
from
October 15, 2026 15:10
04ba73c to
3c2e0cf
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
from
October 15, 2026 15:10
c00bdc9 to
3e0c865
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
from
October 15, 2026 15:58
3c2e0cf to
cc8b607
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
2 times, most recently
from
October 15, 2026 16:36
5bb9819 to
f28baa3
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
2 times, most recently
from
October 15, 2026 18:30
324cdde to
a5a707f
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
from
October 15, 2026 18:30
f28baa3 to
6140e16
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
from
October 15, 2026 18:55
a5a707f to
7bdd040
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
2 times, most recently
from
October 15, 2026 20:07
3d91ccf to
db97dfc
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
from
October 15, 2026 20:07
7bdd040 to
6767538
Compare
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
that referenced
this pull request
Oct 15, 2026
…20952) Stacked PRs: - #19778 — runtime identity foundation (merged) - #19898 — generic WSGI/ASGI/Django request-start event (merged) - #19939 — MicroVM `/run` listener and identity refresh (merged) - This PR #20952 — tracer: rebuild the trace writer (part 1 of #19820) - #20953 — telemetry: rebuild the worker (part 1 of #19821) - #20954 — registers the `/run` listener; runtime-metrics runtime-ID tag (part 1 of #19822) - #20957 — runtime metrics: reset collector baselines (part 2 of #19822) - #20956 — telemetry: discard the old worker and exporter subscription (part 2 of #19821) - #20955 — tracer: drop trace state from before the refresh (part 2 of #19820) Part 1 PRs (#20952, #20953, #20954) can ship first. Part 2 PRs (#20955, #20956, #20957) can ship later. **Release coupling:** #20952 is intended to ship together with #20954. This PR registers the tracer as an identity-refresh subscriber; #20954 registers `maybe_refresh_identity()` on `WEB_REQUEST_STARTING_EVENT`, which is what makes the `/run` request trigger the refresh in production. Do not release #20952 without #20954. ## Description This PR and #20955 split #19820, so the runtime-ID fix can ship before the discard logic. This PR makes the tracer report the new runtime ID after a MicroVM identity refresh. #20955 drops trace state from before the refresh. The native trace exporter copies the runtime ID when it's built (`set_runtime_id(get_runtime_id())` in `_build_base_exporter_builder()`), so after `refresh_identity()` the tracer keeps reporting the snapshot's runtime ID until its writer is rebuilt. The root span's `runtime-id` tag already reads `get_runtime_id()` for each span and needs no change. - `ddtrace/trace/__init__.py` registers `tracer._refresh_runtime_identity` with `on_runtime_identity_refresh()` inside a MicroVM check. - `Tracer._refresh_runtime_identity()` rebuilds the writer the way the post-fork path does (`_recreate(reset_buffer=False, flush_writer=False)`), then stores the tracer metadata again, in a `finally`, so a failed rebuild still records the new runtime ID. A failed rebuild is retried on the next span start. A Rust panic from the rebuild is logged instead of re-raised, because the refresh dispatcher only contains `Exception` and the panic would otherwise abort the `/run` request. The span-start retry contains its own failures in a MicroVM: an `Exception` or Rust panic from the rebuild is logged (rate-limited) and the retry stays pending, so it doesn't fail the app's span start. `KeyboardInterrupt` and `SystemExit` still propagate, and outside a MicroVM a failed post-fork rebuild still raises, as on `main`. - The refresh and `shutdown()` serialize on `_post_fork_lock`. `shutdown()` sets `_shutting_down` under that lock, so it waits for a refresh in flight and stops the writer that refresh installs, and a refresh that starts after shutdown builds nothing. The lock is now reentrant, because the exit-signal handler can run `shutdown()` on the thread that is refreshing. If that handler returns, the refresh sees `_shutting_down` after `_recreate()` and stops the writer it just installed, shutting down the exporter of a `NativeWriter` that never started. - In a MicroVM, `shutdown(timeout)` waits at most `timeout` (clamped at 0) for `_post_fork_lock`, then sets `_shutting_down` either way. Without the bound, an exit signal could deadlock: a refresh on another thread holds `_post_fork_lock` while waiting for the old writer's `_writer_lock`, which the thread running the signal handler holds. A refresh still running after the wait stops the writer it installs, through the same `_shutting_down` check. `shutdown()` without a timeout still waits as before. The span-start retry of a failed refresh (`_ensure_post_fork_writer()`) does the same in a MicroVM: it builds nothing once `_shutting_down` is set, and stops a writer it installs after that wait. Outside a MicroVM the post-fork rebuild is unchanged. The rebuild stops the old writer the normal way, so the old writer still flushes what it buffered. A span finished on another thread after that stop would restart the old writer through the lazy start in `_write_with_client()`, leaving a writer that nothing stops. So the refresh first closes the old writer to new spans, using #19820's MicroVM-only write gate: `_writer_lock` and `_accepting_writes` on `HTTPWriter` and `NativeWriter`, checked in `write()`. `TraceWriter._stop_accepting_writes()` closes the gate without dropping anything. #20955 replaces it with `drop_buffered_traces()`. How a refresh runs, and what a span finished on another thread sees meanwhile: ```mermaid sequenceDiagram participant Run as /run thread participant T as Tracer participant Old as old writer participant New as new writer participant App as span-finishing thread Run->>T: _refresh_runtime_identity(new_id) Note over T: holds _post_fork_lock (reentrant) until the end opt shutdown already started T-->>Run: return, build nothing end T->>Old: _stop_accepting_writes() (under the old writer's _writer_lock) T->>Old: _recreate(reset_buffer=False, flush_writer=False) stops it, flushing what it buffered T->>New: built by recreate() with the new runtime ID T->>T: _store_metadata() in a finally, so the new runtime ID is recorded even if the rebuild failed opt shutdown() ran during the rebuild T->>New: stop the writer it just installed end Note over T: releases _post_fork_lock App->>T: span finishes, aggregator writes the trace alt before the old writer is closed T->>Old: written, and flushed when the old writer stops else after closing, before the swap T->>Old: write() returns without writing, so the closed writer is not restarted else after the swap T->>New: written under the new runtime ID end ``` Nothing registers `maybe_refresh_identity()` until #20954, so merging this PR alone doesn't change runtime behavior. Smaller changes: - `NativeWriter.write()` takes the write gate through the sync-mode flush, as `HTTPWriter.write()` does. Otherwise the refresh could close the writer between the buffering and the flush, and `recreate()` would shut down the exporter that flush then sends through. #19820 keeps the flush outside the gate and instead rechecks `_discarded_by_refresh` before each send; those rechecks belong to #20955. ## Testing `tests/tracer/runtime/test_runtime_id.py`: - `test_tracer_microvm_identity_refresh_recreates_exporter_without_fork_side_effects`: the refresh rebuilds the writer with the post-fork arguments and stores the metadata, without fork side effects. - `test_tracer_microvm_identity_refresh_closes_and_flushes_old_writer`: the old writer sends its buffer once and shuts its exporter down. A later write neither buffers nor restarts it. - `test_tracer_microvm_identity_refresh_after_shutdown_does_not_recreate_writer` - `test_tracer_microvm_identity_refresh_keeps_pending_after_recreate_failure`: a failed rebuild keeps the pending post-fork rebuild and still stores the metadata. - `test_tracer_microvm_identity_refresh_schedules_rebuild_after_recreate_failure`: a failed rebuild is left pending for the next span. `RuntimeError` and `KeyboardInterrupt` propagate; a Rust panic doesn't. - `test_tracer_identity_refresh_contains_native_panic_from_writer_rebuild`: a panic from the rebuild doesn't escape `refresh_identity()`, and other refresh subscribers still run. - `test_tracer_microvm_identity_refresh_retries_failed_writer_recreation_on_next_span`: after a failed rebuild, the next span rebuilds the writer, and the stored metadata carries the refreshed runtime ID. - `test_tracer_shutdown_waits_for_in_flight_identity_refresh`: shutdown during a refresh stops the writer the refresh installs. - `test_tracer_identity_refresh_during_shutdown_does_not_recreate_writer`: a refresh that starts while shutdown runs builds no writer. - `test_tracer_shutdown_on_refreshing_thread_stops_replacement_writer`: shutdown run on the refreshing thread (the exit-signal case) doesn't deadlock, and the replacement writer is stopped, or has its exporter shut down if it never started. - `test_tracer_signal_shutdown_does_not_deadlock_with_refresh_waiting_on_writer_lock`: shutdown run by a thread that holds the writer lock a refresh is waiting on (the cross-thread exit-signal case) returns after its timeout, and the refresh then stops the writer it installed. - `test_tracer_microvm_shutdown_accepts_negative_timeout` - `test_tracer_timed_out_shutdown_stops_writer_from_retried_refresh_rebuild`: a `shutdown(timeout=0)` that finishes while the span-start retry of a failed refresh is rebuilding still gets the rebuilt writer stopped. It fails without the fix. - `test_tracer_post_fork_rebuild_ignores_shutdown_flag_outside_microvm`: outside a MicroVM the post-fork rebuild ignores `_shutting_down`, as before. - `test_tracer_microvm_failed_rebuild_retry_is_contained_in_start_span`: in a MicroVM a span-start retry that fails again with a `RuntimeError` or Rust panic doesn't raise and stays pending, `KeyboardInterrupt` still propagates, and a later span start recovers. It fails without the fix. - `test_tracer_post_fork_rebuild_failure_still_raises_outside_microvm`: outside a MicroVM a failed post-fork rebuild still raises from the span start, as before. - `test_identity_refresh_hook_notifies_global_tracer`, `test_deferred_global_tracer_registers_identity_refresh_callback`, `test_deferred_global_tracer_does_not_register_identity_refresh_callback_outside_microvm`: the global tracer registers only in a MicroVM, including with deferred initialization. `tests/tracer/test_writer.py`: - `test_native_writer_closed_microvm_writer_rejects_writes_without_restarting`, `test_http_writer_closed_microvm_writer_rejects_writes`: a closed writer drops later spans and isn't restarted by them. Spans buffered before the close stay for the final flush. - `test_native_writer_microvm_close_waits_for_in_flight_sync_flush`: closing a sync-mode writer waits for a flush that is sending, and nothing is flushed through the closed writer. - `test_native_writer_stop_accepting_writes_is_noop_outside_microvm`, `test_log_writer_stop_accepting_writes_is_noop` `tests/tracer/test_tracer.py`: - `test_tracer_shutdown_waits_for_in_flight_post_fork_writer_recreation`: shutdown waits for a post-fork writer rebuild on another thread and stops the rebuilt writer. - `test_tracer_shutdown_timeout_does_not_bound_post_fork_wait_outside_microvm`: outside a MicroVM, a shutdown timeout doesn't cut that wait short. Each new concurrency test fails without the change it covers. Local runs (Python 3.14): - `scripts/run-tests --venv 128b106 -- tests/tracer/test_tracer.py tests/tracer/test_writer.py tests/tracer/runtime/test_runtime_id.py`: 316 passed, 13 skipped, 1 xpassed. - After rebasing onto `main` `2ad3da71cb`: `scripts/run-tests -r --venv 128b106 -- tests/tracer/runtime/test_runtime_id.py tests/tracer/test_tracer.py tests/tracer/test_writer.py -k "test_runtime_id or test_writer or shutdown or post_fork"`: 209 passed, 10 skipped, 1 xpassed. - With the shutdown-retry and retry-containment fixes (Python 3.13): `scripts/run-tests --venv 1ef5a52 -- tests/tracer/test_writer.py tests/tracer/runtime/test_runtime_id.py tests/tracer/test_tracer.py`: 320 passed, 13 skipped, 1 xpassed. - `scripts/lint fmt` and `scripts/lint typing` on the changed files: passed. Not done yet: measuring `/run` in a MicroVM with the agent stopped (see Risks). ## Risks Outside a MicroVM the callback isn't registered and `_writer_lock` is `None`, so `write()` takes the lock-free path. The one change there: `shutdown()` now takes `_post_fork_lock` once, so a shutdown that races a post-fork writer rebuild on another thread waits for it and then stops the rebuilt writer, instead of leaving it running. The shutdown timeout doesn't bound that wait outside a MicroVM. Inside a MicroVM, once #20954 registers the listener and until #20955 lands: - `/run` waits for the old writer's final flush, because `stop()` joins the writer thread. That's one send plus `exporter.shutdown()`, which can take up to 3 s. With a healthy agent that's milliseconds. With an unreachable agent it's the send's retries plus up to 3 s. - Traces and client stats buffered before `/run` are sent under the snapshot's runtime ID, once by each restored instance. - Spans that other threads finish during that window are dropped. - Spans open across `/run` keep the old root `runtime-id` tag and go to the new writer. - A writer rebuild running at the same time as the refresh (`Tracer.configure()`, LLMObs) can still install a writer built for the old runtime ID. In supported usage these rebuilds run at startup, before the snapshot. Remote config, whose AppSec activation calls `tracer.configure()` at runtime, isn't supported in MicroVMs. #20955 serializes writer swaps with the refresh. - If `_recreate()` raises, the closed old writer stays installed and drops spans until a span start retries the rebuild successfully. While the rebuild keeps failing, every span start retries it, and the warning is rate-limited to one a minute. - In the exit-signal deadlock case, `shutdown(timeout)` can take up to about 2 × `timeout`: the bounded wait for an in-flight refresh, then the normal flush. If it gives up waiting while the span-start retry of a failed refresh (`_ensure_post_fork_writer()`) holds the lock, that retry stops the writer it installs. - A Rust panic from the native metadata write still reaches `/run`, as it does from `__init__` and after a fork. The write runs after the rebuild, so it can't skip it. Unchanged from `main`, and not specific to the refresh: - An exit signal that arrives while a writer rebuild is inside the old writer's `stop()` deadlocks on that writer's `_service_lock`. `Tracer.configure()` and the post-fork rebuild have the same exposure. - Outside a MicroVM, a post-fork rebuild interrupted by a same-thread shutdown whose signal handler returns can still install a writer after shutdown. ## Additional Notes Each part 2 tip has the same tree as its original PR: #20955 matches #19820, #20956 matches #19821, and #20957 matches #19822. So the fixes made on this PR and on the originals are all in part 2. Left out on purpose: - A cap on the `/run` wait. The telemetry worker's `stop()` (#20953) also blocks `/run`, and this code sets no timeout on it, so capping the trace writer alone wouldn't bound `/run`. - #19820's span-aggregator writer-swap lock. Without the discard, it would be held across the old writer's final flush and block every span start and finish. - #19820's detach of the context active at `/run`. ## Reference - [RFC](https://docs.google.com/document/d/17lde4Zak2YRBuDvf32KFanXj9TdpOv24GYaEY8swA1w/edit?tab=t.0#heading=h.wwu37decnk8j) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: tianning.li <tianning.li@datadoghq.com>
litianningdatadog
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Non-MicroVM worker transitions can cache stale metric contexts and cause reporting calls to dereference a cleared worker.
2 open findings
What changed in this PR
Strengthens telemetry worker replacement during MicroVM identity refresh, building on #20953.
Changes:
- Serializes MicroVM worker access and synchronizes exporter subscriptions.
- Adds discard support, retaining the flushing
stop()fallback until nativedrop()is available. - Adds concurrency coverage; includes inherited trace-isolation changes from #20955.
| File | Description |
|---|---|
| tests/tracer/test_writer.py | Tests exporter lifecycle and worker subscriptions. |
| tests/tracer/runtime/test_runtime_id.py | Tests refresh isolation and stuck sends. |
| tests/telemetry/test_writer.py | Tests worker locking, replacement, and discard. |
| tests/appsec/sca/test_telemetry.py | Updates mocked writer lock setup. |
| releasenotes/notes/fix-aws-lambda-microvm-stale-traces-a26d7bf374c1b2c2.yaml | Inherited trace-isolation release note. |
| ddtrace/internal/writer/writer.py | Updates exporter subscriptions and cleanup. |
| ddtrace/internal/telemetry/writer.py | Synchronizes worker access and replacement. |
| ddtrace/internal/telemetry/noop_writer.py | Updates no-op subscription methods. |
| ddtrace/_trace/tracer.py | Inherited stale-span and context isolation. |
| ddtrace/_trace/span.py | Adds identity-generation key. |
| ddtrace/_trace/processor/__init__.py | Isolates aggregation across identity refreshes. |
| ddtrace/_trace/context.py | Tracks context identity generations. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| else: | ||
| metadata = None | ||
| worker.add_dependency(name, version, metadata) | ||
| self._worker.add_dependency(name, version, metadata) |
| # caller read it, caching this context would hand it to the new worker. | ||
| if worker is self._worker: | ||
| self._metric_contexts[key] = context | ||
| self._metric_contexts[key] = context |
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
from
October 15, 2026 21:04
db97dfc to
df094a9
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
2 times, most recently
from
October 15, 2026 21:44
285b846 to
7195e86
Compare
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-1-telemetry-identity-refresh
branch
from
October 15, 2026 21:44
df094a9 to
da7829a
Compare
Part 1 stops the old trace writer the normal way. The old writer still flushes traces and client stats buffered before the refresh under the snapshot's runtime ID, /run waits for that final flush, and spans that were open across /run join the new runtime. This change replaces that with a discard: - Writers get drop_buffered_traces(). In MicroVMs it closes the write gate and drops the native exporter without shutting it down. A discarded writer skips its final flush, never starts another send, and its stop() does not wait for a send stuck in retries. - The span aggregator publishes a (generation, runtime ID) identity. A refresh bumps the generation, drops the aggregator's trace buffer and discards the writer under the aggregator lock. Other writer swaps take the same lock. - Spans and retained Context objects from an older generation are rejected before they reach the new writer. The context that was active at /run is detached, also when the writer rebuild fails. - A sync-mode write is buffered under the aggregator lock and sent after the lock is released, so a send stuck in retries cannot block span starts or /run. This replaces part 1's _stop_accepting_writes() with drop_buffered_traces(). It also switches part 1's refresh from _recreate(reset_buffer=False, flush_writer=False) to _recreate(reset_buffer=True, drop_buffered_traces=True). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…icroVM refresh Part 1 rebuilds the worker without a worker access lock and leaves the trace exporter's subscription as it was. This change keeps telemetry off the worker built for the old runtime ID: - In MicroVMs, lifecycle methods, data paths and metric recording hold a worker access lock from worker lookup through the native call, so nothing is recorded into the old worker once the refresh starts. Outside MicroVMs each method calls a lock-free twin, and the four add_*_metric methods keep their inline hot path. The lock replaces part 1's single worker reads, its metric-context cache check (and the metric lock enable() publishes the worker under) and its replay snapshots. Because of it, _report_endpoints() iterates over a snapshot, and add_log() formats before taking the lock. - _get_shared_worker() rebuilds a worker still bound to an older runtime ID and hands out none if that fails, so the trace exporter never attaches to a stale worker. - _subscribe_worker_changes() takes the worker the exporter was built with and syncs a subscriber that missed a replacement. In MicroVMs NativeWriter._create_exporter() publishes the exporter before subscribing, so _downgrade() reads the old exporter before calling it. - _on_telemetry_worker_changed() doesn't wait for a send stuck in retries. It parks the worker, and the next send applies it under the exporter lock. - The trace exporter unsubscribes when it is torn down (shutdown_exporter(), on_shutdown(), __del__) and when #19820's refresh drops it (_drop_exporter() and the dropped path of on_shutdown()). - _discard_worker() uses the native worker's non-flushing drop() when it has one. It falls back to stop(), which flushes once under the previous runtime ID. The bundled TelemetryWorker does not expose drop() yet, so the fallback is what runs today. Tests return to #19821's versions, and the end-to-end test now checks that the tracer refresh with telemetry does not wait for a stuck send. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
litianningdatadog
force-pushed
the
tianning.li/3-4-2part-2-discard-stale-telemetry
branch
from
October 15, 2026 22:07
7195e86 to
6dd7ba4
Compare
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.
Stacked PRs:
/runlistener and identity refresh/runlistener; runtime-metrics runtime-ID tag (part 1 of chore(runtime): refresh runtime metric identity and collector state #19822)Part 1 PRs (#20952, #20953, #20954) can ship first. Part 2 PRs (#20955, #20956, #20957) can ship later.
Description
This PR is part 2 of #19821, on top of #20953. Its tip has exactly the code of #19821.
Review the last commit only. The first commit repeats #20955's commit, unchanged, so this branch reaches #19821's code without a merge commit. It drops out when this branch is rebased after #20955 lands.
This PR doesn't drop stale telemetry yet. The bundled native
TelemetryWorkerhas nodrop()(onlyTraceExporterdoes), so_discard_worker()always takes itsstop()fallback. That flushes the old worker's queue once under the previous runtime ID, exactly as #20953 does. Thedrop()branch is a hook that takes effect once the native worker exposesdrop().The last commit keeps telemetry off the worker built for the old runtime ID, and replaces #20953's lock-free workarounds with a lock:
_without_locktwin, and the fouradd_*_metricmethods keep their inline hot path.disable(),_report_dependencies(),app_started()and the integration, product and configuration updates take the worker access lock instead of fix(telemetry): rebuild the worker on MicroVM identity refresh #20953's MicroVM enable lock. In a MicroVM they are the same lock.add_log()now waits for this lock.add_log()formats before taking the lock._downgrade()can rebuild the trace exporter on the writer thread, so the exporter can ask for a worker before the telemetry writer has refreshed._get_shared_worker()rebuilds a worker still bound to an older runtime ID and hands out none if that fails, so the exporter never attaches to a stale worker._subscribe_worker_changes(callback, expected_worker, late_callback)syncs a subscriber that missed a replacement. In a MicroVMNativeWriter._create_exporter()publishes the exporter before subscribing it, so_downgrade()now reads the old exporter before calling_create_exporter()._on_telemetry_worker_changed()doesn't wait for a send stuck in retries. It parks the worker, and the next send applies it under the exporter lock.shutdown_exporter(),on_shutdown(),__del__) and when fix(tracer): drop trace state from before a MicroVM identity refresh #20955's refresh drops it (_drop_exporter()and the dropped-exporter path ofon_shutdown()).drop()._discard_worker()discards the old worker with the native worker's non-flushingdrop()when the worker has one, and falls back tostop()otherwise. The fallback logs at debug level, because it runs on every MicroVM refresh.NoOpTelemetryWritergets a no-op_unsubscribe_worker_changes()and the new_subscribe_worker_changes()signature.Testing
tests/telemetry/test_writer.py:test_microvm_metric_recording_serializes_identity_refresh: a refresh can't replace the worker between metric lookup andadd_point(). It replaces fix(telemetry): rebuild the worker on MicroVM identity refresh #20953'stest_metric_context_registered_on_replaced_worker_is_not_cached.test_microvm_shared_worker_handout_rebuilds_stale_worker,test_microvm_shared_worker_handout_withholds_worker_when_drop_fails: the exporter never gets a worker built for an older runtime ID.test_microvm_identity_refresh_drops_worker_without_stop,test_microvm_identity_refresh_discard_failure_propagates: thedrop()branch, and a faileddrop()keeps the old worker and is raised.test_worker_subscription_initial_sync_is_microvm_only,test_microvm_worker_subscription_syncs_replacement,test_microvm_worker_subscription_can_sync_a_local_target,test_non_microvm_worker_subscription_does_not_use_late_callback,test_worker_subscription_can_be_removedtest_report_endpoints_tolerates_concurrent_registrationtest_microvm_identity_refresh_contains_native_panic_and_keeps_stale_worker,test_microvm_periodic_contains_native_panic_from_stale_worker_rebuild: switched fromstop()todrop(), the discard this PR adds.tests/tracer/test_writer.py:test_native_writer_telemetry_worker_change_during_send,test_native_writer_telemetry_worker_change_applies_immediately_when_idle,test_native_writer_keeps_worker_change_published_while_send_consumes_pending: a stuck send doesn't block the re-point in a MicroVM, and no worker change is lost.test_microvm_exporter_late_subscription_updates_local_exporter,test_microvm_recreated_exporter_follows_refresh_before_caller_publishes,test_microvm_downgrade_shuts_down_original_exportertests/tracer/runtime/test_runtime_id.py:test_tracer_microvm_refresh_with_telemetry_does_not_wait_for_stuck_send: end to end, a refresh with telemetry enabled and the trace exporter stuck insend()finishes in under 1 s.Local runs (Python 3.13):
telemetry(venv7dec5d4) ontests/telemetry/test_writer.py tests/telemetry/test_dependency.py: 165 passed.appsec::sca(venv1a7f51a) ontests/appsec/sca/: 127 passed.tracer(venv1ef5a52) ontests/tracer/test_writer.py tests/tracer/runtime/test_runtime_id.py tests/tracer/test_tracer.py: 362 passed, 13 skipped, 1 xpassed.telemetry154 passed.scripts/lint styleon the changed files andscripts/lint typingon the changedddtrace/files: passed.Risks
Outside a MicroVM:
if self._worker_access_lock:check per call, plus subscription bookkeeping when a trace exporter is built or torn down.Inside a MicroVM:
stop(), which blocks until it has drained.drop()branch doesn't run until the nativeTelemetryWorkerexposesdrop(), so today a refresh still flushes the old worker once under the previous runtime ID.Additional Notes
No release note for this PR's own commit. The release note in this PR's diff comes from #20955's repeated commit and goes away on rebase.
Reference
🤖 Generated with Claude Code