chore(symbol-db): refresh metadata after identity refresh - #19824
litianningdatadog wants to merge 1 commit into
Conversation
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 0b9342c | Docs | View more details | Give us feedback! |
BenchmarksBenchmark execution time: 2026-08-23 22:26:19 Comparing candidate commit 26b013a in PR branch Found 0 performance improvements and 11 performance regressions! Performance is the same for 610 metrics, 10 unstable metrics.
|
d59e112 to
16a5332
Compare
26b013a to
82f7a1a
Compare
16a5332 to
8dd7e8e
Compare
82f7a1a to
8211e54
Compare
cde3045 to
a0e3c42
Compare
8211e54 to
41efe78
Compare
Codeowners resolved asResolved from the full PR diff against |
Dependency direction analysis
|
Circular import analysis
|
There was a problem hiding this comment.
Pull request overview
Refreshes Symbol DB metadata after runtime identity changes so post-/run events use the current runtime ID.
Changes:
- Registers identity-change callbacks to rebuild upload metadata.
- Adds subprocess coverage for runtime and upload ID refreshes.
- Requires synchronization between explicit metadata refreshes and concurrent uploads.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Summary |
|---|---|
tests/internal/symbol_db/test_symbols.py |
Tests metadata updates after identity refresh. |
ddtrace/internal/symbol_db/symbols.py |
Refreshes Symbol DB metadata; concurrent refresh/upload access needs synchronization. |
Suppressed comments (3)
ddtrace/internal/symbol_db/symbols.py:567
ScopeContextcan be constructed from the Remote Config poller's worker thread because Symbol DB is enabled lazily, whilerefresh_identity()can run on a request thread.on_runtime_id_change()adds to a global set while_refresh_runtime_id()iterates that set without synchronization; if installation overlaps a refresh, Python can raiseRuntimeError: Set changed size during iteration, aborting the identity-refresh path. Make callback registration/dispatch synchronized, or register this callback before those operations can run concurrently.
on_runtime_id_change(self._on_identity_refresh)
ddtrace/internal/symbol_db/symbols.py:567
- This stores a bound
ScopeContextmethod in the process-global runtime callback set, butBaseModuleWatchdog.uninstall()does not remove it. The Symbol DB RC path can uninstall and reinstall the uploader, so each cycle leaves another retired context subscribed and retained; every later identity refresh then iterates and resets all of those stale contexts. Add callback unregistration to the lifecycle or register one stable callback that resolves the current uploader instance.
on_runtime_id_change(self._on_identity_refresh)
ddtrace/internal/symbol_db/symbols.py:567
- on_runtime_id_change() is invoked by both refresh_identity() and the runtime module's post-fork hook. Since _reset_on_fork is still registered directly with forksafe above, every fork child now regenerates the upload ID and resets the batch counter twice: once through _on_identity_refresh and once through the direct fork hook. Keep a single reset path (or make the notifications distinguishable) so fork initialization does not perform duplicate state resets.
on_runtime_id_change(self._on_identity_refresh)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def _on_identity_refresh(self, new_runtime_id: str) -> None: | ||
| # Same rebuild as _reset_on_fork(): runtimeId is baked into _event_data, so it must be | ||
| # refreshed here too or every batch keeps reporting the pre-refresh snapshot's ID. | ||
| self._reset_on_fork() |
41efe78 to
0b9342c
Compare
|
close it as it is out of the scope |
Stacked PRs:
Description
Split from #19780.
Symbol DB metadata includes runtime identity. Refresh that metadata after runtime identity changes so Symbol DB events emitted after a MicroVM
/runrefresh use the new runtime id.Testing
scripts/lint format_check ddtrace/internal/symbol_db/symbols.py tests/internal/symbol_db/test_symbols.pygit diff --checkStack
Draft split branch. Stacked on #19816.