Conversation
![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: 95de9cf28c
ℹ️ 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".
| if !featureutils.IsDataPlaneEnabled(dda, ddaSpec, f.defaultDataPlaneEnabled) { | ||
| f.logger.Info("The checkrunner feature requires the dataPlane feature.") | ||
| } |
There was a problem hiding this comment.
When Check Runner is enabled while Data Plane remains at its default false or is explicitly disabled, this branch only emits a log and Configure still requires solely AgentCheckRunnerContainerName. The optimized pod therefore omits the agent-data-plane container; the later AddEnvVarToContainer call silently finds no target, while ACR continues sending to localhost:5105, leaving its checks unprocessed. Require/enable the Data Plane component here or reject this configuration rather than proceeding.
Useful? React with 👍 / 👎.
tobz
left a comment
tobz
left a comment
There was a problem hiding this comment.
Left a note about where ADP configurations should be placed, container-wise, to ensure the proper flow of configuration data.
| // ADP must serve the Checks IPC endpoint ACR is pointed at. | ||
| adpEnvVars := mgr.EnvVarMgr.EnvVarsByC[apicommon.AgentDataPlaneContainerName] | ||
| assert.Contains(t, adpEnvVars, dataPlaneChecksEnabledEnvVar, "DD_DATA_PLANE_CHECKS_ENABLED should be set on the agent data plane so it serves the Checks IPC source") | ||
| assert.NotContains(t, agentEnvVars, dataPlaneChecksEnabledEnvVar, "DD_DATA_PLANE_CHECKS_ENABLED is an ADP setting and should not be set on the core agent") | ||
|
|
There was a problem hiding this comment.
We actually don't want to do this: the goal is that all configuration flows through the Core Agent, so we should set these configurations on the Core Agent directly.
There was a problem hiding this comment.
Ah, good point! Addressed with: dec4ce0.
Then I got bitten by the config schema because it didn't know the keys!
This branch has not been deployed
2 participants
What does this PR do?
Enable the ADP check source when ACR is enable.
Motivation
ACR forward checks events to ADP, without the ADP check source they aren't processed.
Additional Notes
Minimum Agent Versions
Are there minimum versions of the Datadog Agent and/or Cluster Agent required?
Describe your test plan
Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel