Repository navigation
Conversation
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".
Overlapping worker processes in Azure App Service run several JVMs that share one configured log file, and each new JVM truncates it, so the old JVM's last lines are lost. In an overlapped IIS recycle, a new JVM wiped the old JVM's log, including its shutdown. Copy the .NET tracer: - Write to <name>-<pid>.<ext> instead of the configured file. - Roll over at DD_MAX_LOGFILE_SIZE bytes (default 10 MB). - At startup, delete this tracer's own per-process files last written more than DD_TRACE_LOGFILE_RETENTION_DAYS ago (default 32, 0 turns cleanup off), stopping quietly at the first failure. Only active when DD_AZURE_APP_SERVICES is set.
This comment has been minimized.
This comment has been minimized.
Contributor
🟢 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. |
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.
AasPerProcessLogFile had no tests, so the agent-logging coverage check failed. The new tests check that, in Azure App Service only, the tracer writes to <name>-<pid>.<ext> next to the configured file. They also check that it falls back to standard error when that file cannot be opened, rolls over to numbered files at DD_MAX_LOGFILE_SIZE (0 turns rollover off), and appends to an existing file instead of truncating it. Cleanup is checked too: it deletes only this tracer's per-process files older than DD_TRACE_LOGFILE_RETENTION_DAYS (0 turns it off), and it stops quietly when a file cannot be deleted. The class is left out of the instruction coverage rule. Its PID lookup has one path for Java 8 and another for Java 9+, and coverage runs on only one JVM, so one path always shows as missed. The class can't use PidHelper, because PidHelper sets up an slf4j logger and this code runs while the tracer's logger is starting. The branch rule still covers the class, and both rules still cover RollingFileOutputStream.
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
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.
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. It's stacked on #12898, so the diff here is the 6 commits on top of that branch. Each commit message has the details.
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.<name>-<pid>.<ext>, which rolls over atDD_MAX_LOGFILE_SIZE(10 MB by default). At startup a background thread deletes this tracer's own per-process files older thanDD_TRACE_LOGFILE_RETENTION_DAYS(32 by default, 0 turns cleanup off). These match the .NET tracer's settings.Motivation
On Azure App Service (Windows), an overlapped IIS recycle leaves Java apps without telemetry until someone restarts them (SVLS-9057, escalation SLES-2826). The old and new worker run at the same time, and both tracers supervise
datadog-trace-agent.exeanddogstatsd.exeon the same install-wide named pipes.We reproduced the overlap in App Service. Before these changes:
JavaBootstrapper, four in our runs, and each one starts and stops its own agents.ProcessSupervisorreset its fault count on every start, so an agent that kept exiting restarted every 2 s forever, while a start that kept throwing stopped retrying after 5 attempts. Once it started an agent, it blocked inwaitFor(), so a hung agent was never checked.With these commits and the StatsD commits from #12898 in a repro build, both agents failed one health check while starting and then passed, with no restart. When the old worker's agents exited, a JVM in the new worker restarted both within 1 s. Traces came back about 2 min 27 s after the recycle started, and about 1 min 50 s of that was JVM restarts in the new worker. In a run with the per-process log file, the old JVM's file was still there after the recycle.
Additional Notes
Design choices and known gaps:
ProcessHandleby reflection, with a JMX fallback on Java 8, becausePidHelpercan't be used safely during logging startup. Its cleanup runs on a plain daemon thread.checkConfigurations: it callsEnvironmentVariables.getdirectly, andDD_MAX_LOGFILE_SIZEandDD_TRACE_LOGFILE_RETENTION_DAYSaren't inmetadata/supported-configurations.json. If the team wants the log file, this gets fixed here. Registering the keys now would add public configuration ahead of review.AasPerProcessLogFileis left out of agent-logging's instruction coverage rule. Its PID lookup has separate Java 8 and Java 9+ paths, and no single JVM runs both. The branch rule still applies to it. If reviewers prefer, the PID lookup can move into its own small class, and only that class would be excluded./infocounts as healthy./infofrom an agent that another worker started causes one duplicate start. The duplicate loses the pipe race and exits./inforequest to a hung agent stays blocked until that agent exits. Later checks fail without sending another request.Tests:
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.AasPerProcessLogFileTestcovers the file name, rollover, and retention cleanup. The pipe health checks and the busy-pipe check ran in the App Service repro on Windows and have no unit tests. Per-process pipe names, so that overlapping workers never share agents, are the planned follow-up and aren't in this PR.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