fix(datadog-agent-commons): Survive health check network partition at boot - #2111
Conversation
Binary Size Analysis (Agent Data Plane)Baseline: f482d34 · Comparison: b4fe46c · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (5)Experiments configured
Bounds Checks: ✅ Passed (5)
ExplanationA change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression ( |
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The PR correctly moves the health check into the retry block to handle transient network partitions at boot. The change is minimal and sound: it makes the health check retryable (up to 10 attempts with 2-second backoff) rather than a single attempt, fixing the specific bug where a network partition after TCP connect but before the health check RPC would terminate ADP. No behavioral regressions found.
📊 Validated against 5 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit b04c5f8 · What is Autotest? · Any feedback? Reach out in #autotest
5e60473 to
35f3d41
Compare
b04c5f8 to
7723112
Compare
blt
changed the title
35f3d41 to
5f369f8
Compare
7723112 to
4e6c19c
Compare
![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: 4e6c19ce36
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // this first RPC stream, so retrying here keeps a boot-time blip from failing client construction. | ||
| let mut secure_client = | ||
| AgentSecureClient::new(service.clone()).max_decoding_message_size(config.grpc_max_message_size()); | ||
| try_query_agent_api(&mut secure_client).await?; |
There was a problem hiding this comment.
When this health check returns Code::Unauthenticated, try_query_agent_api converts it to a GenericError; because the call now runs inside service_builder.retry(config) with no retry predicate, a wrong token is retried for connect_retry_attempts with connect_retry_backoff sleeps before surfacing the same configuration error. In deployments with invalid credentials this delays startup by about 20s by default and logs misleading connection-retry warnings, so exclude unauthenticated health-check failures from this retry path.
Useful? React with 👍 / 👎.
This comment has been minimized.
This comment has been minimized.
fdaee90 to
142ee6c
Compare
4e6c19c to
31ccfef
Compare
31ccfef to
b3c4cec
Compare
At boot time ADP establishes a gRPC connection with core Agent. This connection is done in multiple parts, remote agent registration, hostname fetch, health check etc. Most of these are managed by RemoteAgentClient and are retried, however, the health check is attempted only once post-connection. This means that a network partition between a successful connection and the health check terminates ADP. This commit moves the health check into the same retry scheme as the rest of the boot sequence. REF SMPTNG-766
b3c4cec to
b4fe46c
Compare
There was a problem hiding this comment.
More details
PR moves health check into the retry loop, allowing transient RPC failures during boot to be retried instead of failing immediately. The change is architecturally sound: the health check validates connection and authentication once per retry attempt, and the returned service is unused and in good state for client creation. All error paths are properly handled, and the API remains unchanged for callers.
📊 Validated against 5 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit b3c4cec · What is Autotest? · Any feedback? Reach out in #autotest
… boot (#2111) ## Summary <!-- Please provide a brief summary about what this PR does. This should help the reviewers give feedback faster and with higher quality. --> At boot time ADP establishes a gRPC connection with core Agent. This connection is done in multiple parts, remote agent registration, hostname fetch, health check etc. Most of these are managed by RemoteAgentClient and are retried, however, the health check is attempted only once post-connection. This means that a network partition between a successful connection and the health check terminates ADP. This commit moves the health check into the same retry scheme as the rest of the boot sequence. ## Change Type - [x] Bug fix - [ ] New feature - [ ] Non-functional (chore, refactoring, docs) - [ ] Performance ## How did you test this PR? <!-- Please how you tested these changes here --> ## References <!-- Please list any issues closed by this PR. --> REF SMPTNG-766 <!-- - Closes: <issue link> --> <!-- Any other issues or PRs relevant to this PR? Feel free to list them here. --> 06d847c
Summary
At boot time ADP establishes a gRPC connection with core Agent. This
connection is done in multiple parts, remote agent registration,
hostname fetch, health check etc. Most of these are managed by
RemoteAgentClient and are retried, however, the health check is attempted
only once post-connection. This means that a network partition between
a successful connection and the health check terminates ADP.
This commit moves the health check into the same retry scheme as the
rest of the boot sequence.
Change Type
How did you test this PR?
References
REF SMPTNG-766