Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PROCS-3710] Add ability to run Processes features on Core Agent #1121

Closed
wants to merge 18 commits into from

Conversation

daniel-taf
Copy link
Collaborator

@daniel-taf daniel-taf commented Mar 14, 2024

What does this PR do?

This PR adds the ability to run all Processes features on the core agent. A config type, ProcessesRunInCoreAgent, is added to all Processes feature types to facilitate this. Because each feature can have its own value set, the source of truth is handled with the following precedence: Live Processes > Live Containers > Process Discovery.

Motivation

What inspired you to submit this pull request?

Additional Notes

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

  • Agent: v7.52.0
  • Cluster Agent: N/A

Describe your test plan

Each feature was tested in both configurations (core agent and process agent) on a local kind cluster.
Live Processes Example:
Process Agent

  features:
    liveProcessCollection:
      enabled: true

Results in:

❯ kubectl get pod datadog-agent-9jvw9 -o json | jq '.spec.containers[].name'
"agent"
"trace-agent"
"process-agent"
❯ kubectl logs datadog-agent-9jvw9 --container process-agent | grep checks
2024-03-21 19:28:00 UTC | PROCESS | INFO | (pkg/process/runner/runner.go:286 in Run) | Starting process-agent with enabled checks=[process rtprocess]

Core Agent

  features:
    liveProcessCollection:
      enabled: true
      runInCoreAgent:
        enabled: true

Results in:

❯ kubectl get pod datadog-agent-4hr8f -o json | jq '.spec.containers[].name'
"agent"
"trace-agent"
❯ kubectl logs datadog-agent-4hr8f --container agent | grep checks
2024-03-21 19:56:09 UTC | CORE | INFO | (pkg/process/runner/runner.go:286 in Run) | Starting process-agent with enabled checks=[process rtprocess]

Checklist

  • PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • PR has a milestone or the qa/skip-qa label

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request does not contain a valid label. Please add one of the following labels: bug, enhancement, refactoring, documentation, tooling, dependencies

@daniel-taf daniel-taf added the enhancement New feature or request label Mar 14, 2024
@daniel-taf daniel-taf added this to the v1.4.0 milestone Mar 14, 2024
@daniel-taf daniel-taf changed the title Add ability to run Live Processes feature on core agent Add ability to ru Processes features on core agent Mar 19, 2024
@daniel-taf daniel-taf changed the title Add ability to ru Processes features on core agent Add ability to run Processes features on core agent Mar 19, 2024
@daniel-taf daniel-taf changed the base branch from main to daniel.tafoya/orchestrator-process-agent-container-fix March 19, 2024 19:53
@daniel-taf daniel-taf changed the title Add ability to run Processes features on core agent Add ability to run Processes features on Core Agent Mar 19, 2024
@daniel-taf daniel-taf force-pushed the daniel.tafoya/processes-run-in-core branch from fea8213 to 27eb10e Compare March 20, 2024 19:24
@daniel-taf daniel-taf changed the base branch from daniel.tafoya/orchestrator-process-agent-container-fix to main March 20, 2024 19:30
@daniel-taf daniel-taf changed the base branch from main to daniel.tafoya/orchestrator-process-agent-container-fix March 20, 2024 19:31
@codecov-commenter
Copy link

codecov-commenter commented Mar 20, 2024

Codecov Report

Attention: Patch coverage is 93.79310% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 59.21%. Comparing base (6571686) to head (2f858dd).

Additional details and impacted files

Impacted file tree graph

@@                                    Coverage Diff                                     @@
##           daniel.tafoya/orchestrator-process-agent-container-fix    #1121      +/-   ##
==========================================================================================
+ Coverage                                                   58.98%   59.21%   +0.22%     
==========================================================================================
  Files                                                         174      174              
  Lines                                                       21387    21511     +124     
==========================================================================================
+ Hits                                                        12616    12737     +121     
- Misses                                                       8004     8006       +2     
- Partials                                                      767      768       +1     
Flag Coverage Δ
unittests 59.21% <93.79%> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
apis/datadoghq/v2alpha1/datadogagent_types.go 100.00% <ø> (ø)
...ollers/datadogagent/feature/liveprocess/feature.go 93.27% <100.00%> (+6.69%) ⬆️
...lers/datadogagent/feature/livecontainer/feature.go 88.23% <90.69%> (+0.35%) ⬆️
...s/datadogagent/feature/processdiscovery/feature.go 85.00% <91.52%> (+11.53%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6571686...2f858dd. Read the comment docs.

@celenechang celenechang modified the milestones: v1.4.0, v1.6.0 Mar 21, 2024
@daniel-taf daniel-taf changed the title Add ability to run Processes features on Core Agent [PROCS-3710] Add ability to run Processes features on Core Agent Mar 21, 2024
Comment on lines +131 to +141
containerName := apicommonv1.ProcessAgentContainerName
runInCoreAgent := f.runInCoreAgentCfg != nil && f.runInCoreAgentCfg.enabled
if runInCoreAgent {
containerName = apicommonv1.CoreAgentContainerName
}
runInCoreAgentEnvVar := &corev1.EnvVar{
Name: apicommon.DDProcessConfigRunInCoreAgent,
Value: apiutils.BoolToString(&runInCoreAgent),
}
managers.EnvVar().AddEnvVarToContainer(apicommonv1.ProcessAgentContainerName, runInCoreAgentEnvVar)
managers.EnvVar().AddEnvVarToContainer(apicommonv1.CoreAgentContainerName, runInCoreAgentEnvVar)
Copy link
Collaborator Author

@daniel-taf daniel-taf Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any good place to put util functions that are shared between certain features? Lot of shared code could be reduced.

@daniel-taf daniel-taf marked this pull request as ready for review March 21, 2024 21:59
@daniel-taf daniel-taf requested review from a team as code owners March 21, 2024 21:59
Comment on lines +235 to +237
// RunInCoreAgent controls enablement of running the feature in the core agent.
// +optional
RunInCoreAgent *ProcessesRunInCoreAgent `json:"runInCoreAgent,omitempty"`
Copy link
Collaborator Author

@daniel-taf daniel-taf Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be better to have one top level configuration for this to avoid having to handle inconsistent values as it adds a lot of logic. Is there a good way to do that? It seemed to be against the standard convention to do so.

Copy link
Contributor

@hestonhoffman hestonhoffman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple small docs edits

@@ -96,7 +96,9 @@ spec:
| features.kubeStateMetricsCore.conf.configMap.name | Name is the name of the ConfigMap. |
| features.kubeStateMetricsCore.enabled | Enabled enables Kube State Metrics Core. Default: true |
| features.liveContainerCollection.enabled | Enables container collection for the Live Container View. Default: true |
| features.liveContainerCollection.runInCoreAgent.enabled | Enabled enables running parent feature in core agent (experimental) Default: false |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| features.liveContainerCollection.runInCoreAgent.enabled | Enabled enables running parent feature in core agent (experimental) Default: false |
| features.liveContainerCollection.runInCoreAgent.enabled | Enabled enables running parent feature in core Agent (experimental). Default: false |

| features.liveProcessCollection.enabled | Enabled enables Process monitoring. Default: false |
| features.liveProcessCollection.runInCoreAgent.enabled | Enabled enables running parent feature in core agent (experimental) Default: false |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| features.liveProcessCollection.runInCoreAgent.enabled | Enabled enables running parent feature in core agent (experimental) Default: false |
| features.liveProcessCollection.runInCoreAgent.enabled | Enabled enables running parent feature in core Agent (experimental). Default: false |

@@ -124,6 +126,7 @@ spec:
| features.otlp.receiver.protocols.http.enabled | Enable the OTLP/HTTP endpoint. |
| features.otlp.receiver.protocols.http.endpoint | Endpoint for OTLP/HTTP. Default: '0.0.0.0:4318'. |
| features.processDiscovery.enabled | Enabled enables the Process Discovery check in the Agent. Default: true |
| features.processDiscovery.runInCoreAgent.enabled | Enabled enables running parent feature in core agent (experimental) Default: false |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| features.processDiscovery.runInCoreAgent.enabled | Enabled enables running parent feature in core agent (experimental) Default: false |
| features.processDiscovery.runInCoreAgent.enabled | Enabled enables running parent feature in core Agent (experimental). Default: false |

@daniel-taf daniel-taf force-pushed the daniel.tafoya/orchestrator-process-agent-container-fix branch from 9cf434c to 3072e4e Compare March 26, 2024 14:51
Base automatically changed from daniel.tafoya/orchestrator-process-agent-container-fix to main March 26, 2024 18:22
@daniel-taf daniel-taf closed this Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants