Repository navigation
Conversation
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
2254f7d to
0c39b52
Compare
391843b to
1c4bed4
Compare
In Azure App Service the tracer starts and supervises the trace agent and DogStatsD. Overlapping worker processes during platform restarts can leave those agents missing or shared, but starts, exits, and stops were only logged at debug level, so default customer logs could not show what happened. - Log each agent start, exit (with exit code and run time), and stop at info, with the tracer PID and, on Java 9+, the agent PID. - Log when an already-running agent not started by this process is used, and when that agent goes away. - After repeated quick failures, move lifecycle lines to debug until the agent stays up, so a crash loop does not flood the log. - Warn when shutdown leaves an agent running.
ProcessSupervisor reset its fault count on every successful start, so an agent that kept exiting was restarted every 2 seconds forever, while a start that kept throwing gave up for good after 5 attempts. - Forget faults only after the process stays up for 60 seconds. - Back off after each fault, doubling from 2 seconds to a cap of 60 seconds, and never stop retrying. - Move repeated start exceptions to debug after the first few, so a permanently failing start does not log a stack trace every minute.
Once ProcessSupervisor started an agent it blocked in waitFor() until the agent exited, so an agent that hung but stayed alive was never checked or replaced. - While its own agent runs, wait in keep-alive steps of 10 seconds and run the health check after each one. - After 3 failed health checks in a row, stop that agent and go back through the normal start path, after the fault backoff. Log the first failed check and the restart once. - Keep tracking an agent that is still running after a stop request, so a restart never starts a second copy next to it. - Supervisors without a health check keep only waiting for the exit.
On Windows, File.exists() can report false for a named pipe whose instances are all busy. The supervisor then took a busy agent for a missing one and started a competing agent, which lost the pipe race and exited. When File.exists() reports false on Windows, open the pipe once and close it again. Only "cannot find the file" means the pipe is absent; "All pipe instances are busy" and other open errors mean a process is serving it.
The trace agent health check only tested that its pipe existed, so a hung agent that kept its pipe was trusted forever. - After the pipe check passes, send a minimal GET /info over a pipe handle of the check's own, outside the tracer's HTTP clients, with tracing muted. - Treat any HTTP status line as an answer. A busy pipe counts as present. - Pipe reads cannot time out, so send the request on a daemon thread and give up after 2 seconds. While that request is still blocked, later checks fail without sending another one. - DogStatsD never replies, so its check stays "pipe present".
1c4bed4 to
42dd7e4
Compare
The supervisor now health checks its own agent while it runs, restarts it after three failed checks in a row, uses an agent another process started, and never gives up retrying. Only the backoff arithmetic was tested, so coverage of these paths was too low for the build to pass. The new tests start a fake Process from a mocked ProcessBuilder. The fake returns quickly from timed waits, so health checks that normally run ten seconds apart run fast, and each test can choose how the process exits or refuses to stop. They check that the supervisor: - restarts its own process after exactly three failed health checks; - keeps its process when a health check passes before the third failure; - uses a process it did not start, then starts its own once that one goes away (this waits out the real ten second healthy interval); - keeps tracking a process that ignores stop requests instead of starting a second copy; - retries after the process fails to start; - logs the exit code and starts again after the process exits.
1823a62 to
d3b4fd3
Compare
healthCheck(pipe, true) and healthCheck(pipe, false) didn't say what the flag meant at the call site. Use answersOnPipe for the trace agent, which must answer GET /info, and pipePresent for DogStatsD, which never replies on its pipe. No behavior change.
The supervised agents run only on Windows, where Process.destroy() and destroyForcibly() both call TerminateProcess, so the forced second attempt did nothing new. Call destroy() once and wait up to 400 ms, the same total wait as the two 200 ms waits before. A process that is still running after that is kept and tracked as before.
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3b4fd3ff9
ℹ️ 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".
There was a problem hiding this comment.
Localized Windows missing-pipe errors are treated as proof that an agent is running, preventing trace-agent and DogStatsD startup or recovery on affected hosts.
🤖 Bits Code Review · Commit d3b4fd3
- An exit before the process stayed up for 60 s is now a fault whatever its exit code, so it backs off and counts toward quiet logging. The trace agent exits with 0 at once when APM is disabled, and that used to restart about every 100 ms, now with start and exit lines at info. - Lifecycle logs return to info once the running process has stayed up for 60 s, not only after it exits. - A trace agent /info answer that arrives after the 2 s timeout counts at the next check, instead of being dropped by a new request. An agent that always answers in just over 2 s was failing every check.
|
@DataDog review |
There was a problem hiding this comment.
More details
Health-triggered restarts remain restricted to agents started by this tracer, preserving agents adopted from overlapping workers. Confidence is limited to static analysis; Windows named-pipe recovery was not exercised at runtime.
🤖 Bits Code Review · Commit 92c63d2
What Does This Do
This makes the tracer's agent supervisor in Azure App Service recover from overlapping restarts, and makes its lifecycle readable in default-level logs. A followup to #12898.
ProcessSupervisorlogs each agent start, exit (exit code and run time), and stop at info, with the tracer PID and, (on Java 9+) the agent PID. It also logs when it uses an agent that another process started, when that agent disappears, and when shutdown leaves an agent running. After repeated quick failures these lines drop to debug until the agent stays up.GET /infoover a pipe handle of its own, with tracing muted, and waits at most 2 s.Motivation
On Azure App Service (Windows), an overlapped IIS recycle leaves Java apps without telemetry until someone restarts them (SVLS-9057, escalation SLES-2826).
Background. In App Service, the Datadog site extension doesn't run an agent. Each tracer starts
datadog-trace-agent.exeanddogstatsd.exeitself and sends to them over Windows named pipes. The extension picks the two pipe names once, at install, so every worker process on the site uses the same names. Only one process can own a pipe name. An agent started while another agent still holds the name fails to create its pipe, and the trace agent exits at once with "Access is denied". So before starting an agent, a tracer checks whether the pipe already exists, and if it does, it uses the agent that's already there.During an overlapped IIS recycle, the old and new worker run at the same time, and each worker start also runs several JVMs in a row. So several tracers are checking the same pipes and starting agents on them at once. When one of those agents couldn't start, hung, or kept exiting, the tracer's
ProcessSupervisorhandled it badly:%HOME%, so they can't start while Azure Storage is unavailable. Restarts now back off up to 60 s and never stop (change 2).Additionally:
Additional Notes
/infocounts as healthy./inforequest to a hung agent stays blocked until that agent exits. Later checks fail without sending another request./infofrom an agent started by another worker will cause one duplicate start. The duplicate can't create the pipe and exits.unknown, because Java 8 has no public API for it.ProcessSupervisorTestcovers the backoff, the fault-count reset, restarting after failed health checks, adopting an agent it didn't start, and not starting a second agent while one won't stop.dd-trace-javaon GitLab on Windows OS #12311 -- right now there's no windows CI where named pipes could even potentially be created.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: SVLS-9057