Repository navigation
Conversation
github-actions
Bot
added
the
area:tracer
Lewis-E
changed the title
Lewis-E
changed the title
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9397) and master. ✅ No regressions detected |
Lewis-E
changed the title
BenchmarksBenchmark execution time: 2026-10-16 17:18:41 Comparing candidate commit af0e9b7 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 72 metrics, 0 unstable metrics, 9 known flaky benchmarks, 117 flaky benchmarks without significant changes.
|
Lewis-E
marked this pull request as ready for review
October 15, 2026 20:15
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-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
Restart backoff can launch a duplicate agent after another worker has already restored service. Very large configured retry caps can also overflow the delay and permanently terminate the keep-alive.
🤖 Bits Code Review · Commit 1469fd4
![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: 1469fd4f5f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Lewis-E
force-pushed
the
lewis.lewis/svls-9057-agent-lifecycle-logging
branch
2 times, most recently
from
October 16, 2026 02:02
1f1016d to
fc83561
Compare
In Azure App Service the tracer starts and keeps alive the trace agent and DogStatsD. When overlapping worker processes compete during platform restarts, default logs could not show which agent was started, which one exited and why, or that an agent started elsewhere was being reused. - Log the child PID when an agent is started, and its exit code when it exits during startup or later. - Say when the startup check passed only because another instance is running, instead of reporting a successful start. - Warn when an agent is still running but has not opened its named pipe by the end of the startup wait. - Log once when an agent not started by this process is reused, and when the agent in use goes away; move the per-check "no longer running" line to debug. - Fix the argument order of the "Maximum retries" log message.
DFIX-01 (SVLS-9057). If the startup wait timed out while the agent we started was still running, the state stayed ReadyToStart and the next keep-alive pass started another copy without re-checking. - Re-check health on every pass after the first, including after a startup timeout. - Add an Unhealthy state for "the process we started is running but the health check fails", and only start a new process when ours is not running (GetKeepAliveAction). - Log the transitions into and out of Unhealthy once.
DFIX-02 (SVLS-9057). File.Exists on a named pipe can report false while every server instance is busy, which is why the health check needed a process-name fallback. Check the pipe with WaitNamedPipe and a 1 ms timeout instead: success or ERROR_SEM_TIMEOUT/ERROR_PIPE_BUSY means the pipe exists, anything else (ERROR_FILE_NOT_FOUND) means it does not. The check does not connect to the pipe and returns at once.
DFIX-03 (SVLS-9057). The health check passed whenever any process named datadog-trace-agent or dogstatsd was running, so an orphaned, hung, or shutting-down agent from another worker was trusted forever. Drop the process-name fallback: the agent is healthy only while its named pipe exists. The process we started is still tracked by PID (OwnProcessIsRunning), so a slow-binding agent of ours is not started twice. The startup wait now waits for the pipe instead of passing as soon as the child is running.
DFIX-04 (SVLS-9057). After 5 failed starts in a row the keep-alive logged "Maximum retries" and exited for the rest of the worker's life, for example after a burst of failures during a platform restart. Keep retrying instead, waiting 2 s after the first failure and doubling with each further failure up to 60 s (GetRetryDelay). A healthy check resets the count. Rethrow ThreadAbortException in the loop, since the try block is now the loop's only statement (DD0001).
DFIX-05 (SVLS-9057). A trace agent that still held its pipe but had stopped serving requests passed the health check forever. For the trace agent only, the health check now also sends GET /info over a new pipe client stream (same options as the per-request trace stream) and requires an HTTP status line back. The connect is bounded to 500 ms and the whole exchange to 2 s; on timeout the stream is disposed, which aborts the pending read. The request is only sent once the pipe exists, so it runs once per keep-alive pass (10 s) in steady state. DogStatsD never replies, so its check stays "pipe present". The startup wait is now bounded by elapsed time rather than by loop count, since a check can take up to 2 s.
DFIX-06 (SVLS-9057). A hung agent that this tracer started was never replaced, because nothing stopped it and DFIX-01 rightly refuses to start a second copy next to it. When the process this keep-alive started is still running and fails MaxFailedHealthChecks (3) checks in a row, about 30 s at the 10 s keep-alive interval, kill it and start a new one in the same pass. An agent we did not start is never stopped: with no own process running, the action is always Start. If the kill does not take effect within 1 s, keep tracking the process and try again on the next pass rather than start next to it. The restart is logged once.
Log the start attempt at Information again, log exceptions while checking or stopping our own agent as errors or warnings, and log a trace agent that does not answer its health check request at Information, so these show up in customer logs without enabling debug logging.
An agent this process started that keeps failing its health check was restarted about every 32 s forever, because only failed starts backed off. Failed starts and health-check restarts now share one counter and one backoff: 2 s, doubling, up to DD_AAS_AGENT_MAX_RETRY_INTERVAL_SECONDS (default 300). The counter resets only after the agent has stayed healthy for 60 s, so an agent that flaps between healthy and unhealthy keeps backing off. Retries still never stop. Each restart logs the wait and how many failures in a row led to it.
…gerTests Five failures in a row wait 32 s (2, 4, 8, 16, 32); the 60 s cap only applies from the sixth.
- Count an exit of an agent this process started as a failure, even when it happens after the startup wait and before the next check, and wait for the shared retry delay before starting a new one. First starts and reuse of an already-healthy agent stay immediate. - After a retry wait, check health again and reuse an agent another worker started meanwhile instead of starting a duplicate. - Keep per-probe health check exceptions at Debug. Record why the check failed (pipe missing, connect timeout, no response within the timeout, closed, unexpected response) and include that reason in the default-level state-change logs. - Double the retry delay in long so a very large configured cap cannot overflow into a negative delay. - Log PIDs and exit codes as numbers instead of strings. - Tests: retry delay at the largest accepted cap, start delay, a child that exits between checks, and Windows tests of the trace agent check against a pipe server that answers and one that accepts but never answers.
Follow the repository's log level rule for retries: the first failure in a run of failures logs at Warning (or Information for starts and exits), and later attempts in the same run log at Debug. Failed starts are retried with backoff, so they are no longer logged as errors. State changes (unhealthy, healthy again, no longer available, reusing another agent, successfully started) still log once at the default level.
Inline the message templates in both the Warning and Debug branches so the DDLOG004 analyzer sees constants, and add the blank lines SA1513 requires after closing braces.
CA2022 rejects ReadAsync calls whose byte count is ignored. Assert that each read of the probe's request returns data.
Lewis-E
force-pushed
the
lewis.lewis/svls-9057-agent-lifecycle-logging
branch
from
October 16, 2026 13:39
fc83561 to
411614f
Compare
The test pipe servers have no buffer, so a pipe write completes only once the other side reads it. Both new health check tests failed on every Windows framework because of that, not because of the health check: - The answering agent's response write broke with "Pipe is broken" after the probe read the status line and closed the pipe. Ignore that error. - The silent agent never read the request, so the probe's write never completed and no request bytes ever arrived. Read the request first, then stay silent, so the test covers an agent that never answers.
This branch has not been deployed
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
1 participant
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
In Azure App Service,
AgentProcessManagernow:DD_AAS_AGENT_MAX_RETRY_INTERVAL_SECONDS(default 300). The count resets only after the agent has stayed healthy for 60 s. An exit of an agent this process started also counts, even between checks, and after each wait it re-checks health so an agent another worker started meanwhile is reused;GET /infoon its pipe, with the connect and the read both time-limited;Reason for change
See(SLES-2826, tracked in SVLS-9057) -- A customer loses telemetry after Azure restarts. Overlapped IIS recycling runs an old and a new worker process that share the extension's named pipes. Today:
dogstatsd.exethat lost the pipe still counts as healthy by name, so it is never replaced;This picks up from the closed #8874 and addresses its review comments:
dogstatsd_require_listener(datadog-aas-extension#487) the losing DogStatsD exits instead of staying idle.Implementation details
Unhealthystate means "our process is running but failing its check". The pure helpersGetKeepAliveActionandGetRetryDelayhold the decisions.WaitNamedPipeWwith a 1 ms timeout: busy or timeout counts as present, anything else as absent./infocheck opens a newNamedPipeClientStreamper check, connects within 500 ms, and finishes the exchange within 2 s.DD_AAS_AGENT_MAX_RETRY_INTERVAL_SECONDS(product AzureAppService, int, default 300) insupported-configurations.yaml, read throughImmutableAzureAppServiceSettings. A value that isn't positive falls back to the default.Test coverage
New
AgentProcessManagerTestscover the decisions: never starting while our own process runs, restarting at 3 failures, never restarting an agent we did not start, ignoring a same-named process when its pipe is missing, the retry delay with its default, configured, and largest accepted caps, and a child that exits between checks. Windows-only tests run the/infocheck against a real pipe server that answers and one that accepts but never answers, checking the result, that it finishes within the timeout, and that the connection is closed.In an Azure App Service repro (Windows B2, the published extension with this build laid over it, agent 7.84.0 with
dogstatsd_require_listener), an overlapped recycle under load:There were no duplicates and no restart loops.
Other details
DD_AAS_AGENT_MAX_RETRY_INTERVAL_SECONDSif faster recovery matters more than CPU.