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

Support process-agent configuration in datadog.yaml #511

Merged
merged 1 commit into from Feb 15, 2018

Conversation

Projects
None yet
4 participants
@conorbranagan
Copy link
Member

conorbranagan commented Feb 13, 2018

We can do this as of this change being merged: DataDog/datadog-agent#1136

The configuration format is defined in https://github.com/DataDog/datadog-process-agent/blob/master/config/yaml_config.go

# Example: 'image:redis,image:nginx'
default['datadog']['process_agent']['container_blacklist'] = nil
# Whitelist is applied after the blacklist.
default['datadog']['process_agent']['container_whitelist'] = nil

This comment has been minimized.

Copy link
@conorbranagan

conorbranagan Feb 13, 2018

Author Member

Blacklists for containers are handled in the standard Agent 6 way as we share the tagger code.

This comment has been minimized.

Copy link
@sunhay

sunhay Feb 13, 2018

Member

We should also at some point clean up the config blacklist code in the process-agent since it looks like it's never used: https://github.com/DataDog/datadog-process-agent/blob/master/config/config.go#L65


# If only collecting containers ('enable_process_agent' is false but docker is available)
# overrides the collection intervals for the full and real-time check.
default['datadog']['process_agent']['container_interval'] = nil
default['datadog']['process_agent']['rtcontainer_interval'] = nil

This comment has been minimized.

Copy link
@conorbranagan

conorbranagan Feb 13, 2018

Author Member

we're setting the defaults here explicitly so we don't have to worry about nil values in the config

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Feb 13, 2018

Member

alternatively, you could remove the keys that have nil values in the template logic, with something like agent_config[:process_config].reject!{ |k,v| v.nil? }.

Having the defaults defined in the attributes instead of the agent has been a bit of a pain to work with in the past, hence the suggestion

@sunhay

sunhay approved these changes Feb 13, 2018

@conorbranagan conorbranagan force-pushed the conorbranagan:conor/process-agent branch from 4a2336a to b569a91 Feb 13, 2018

@olivielpeau olivielpeau self-assigned this Feb 13, 2018

@@ -82,6 +93,20 @@ agent_config = @extra_config.merge({
# log agent options
log_enabled: node['datadog']['enable_logs_agent']
# process agent options
process_dd_url: node['datadog']['process_agent']['url'],

This comment has been minimized.

Copy link
@truthbk

truthbk Feb 13, 2018

Member

Note: raclette has put there endpoint within the apm_config section. Could we try to be consistent here? Would dd_url under process_config be better? I know this would imply changes to the process-agent, hopefully it's not too late.

This comment has been minimized.

Copy link
@conorbranagan

conorbranagan Feb 13, 2018

Author Member

This is a good idea. I have a PR on the process-agent side in DataDog/datadog-process-agent#109 that will hopefully make it into the RC1. I'll update this with the assumption that it will get merged shortly.

@@ -82,6 +93,20 @@ agent_config = @extra_config.merge({
# log agent options
log_enabled: node['datadog']['enable_logs_agent']

This comment has been minimized.

Copy link
@truthbk

truthbk Feb 13, 2018

Member

we're missing a comma in the line above :)

@conorbranagan conorbranagan force-pushed the conorbranagan:conor/process-agent branch from b569a91 to d474beb Feb 13, 2018

@truthbk
Copy link
Member

truthbk left a comment

Looks good to me! I'll let @olivielpeau hit the green button! Thank you.

@olivielpeau
Copy link
Member

olivielpeau left a comment

@conorbranagan Made a couple of comments, let me know


# If only collecting containers ('enable_process_agent' is false but docker is available)
# overrides the collection intervals for the full and real-time check.
default['datadog']['process_agent']['container_interval'] = nil
default['datadog']['process_agent']['rtcontainer_interval'] = nil

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Feb 13, 2018

Member

alternatively, you could remove the keys that have nil values in the template logic, with something like agent_config[:process_config].reject!{ |k,v| v.nil? }.

Having the defaults defined in the attributes instead of the agent has been a bit of a pain to work with in the past, hence the suggestion

@@ -53,34 +52,6 @@ def conf_template_vars
notifies :restart, 'service[datadog-agent]', :delayed unless node['datadog']['agent_start'] == false
end

template process_agent_config_file do

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Feb 13, 2018

Member

If this code worked for the beta A6 builds, could you keep it (along with the resource datadog_monitor 'process_agent') for now? I can take care of removing it when the Agent 6 is actually GA.

If we remove it now there's no "safe" path to upgrade from A6 beta.9 to rc.1 without breaking the process agent, as no version of this cookbook would fully support both.

I'd like the next version of the cookbook (2.14.0) to support both beta.9 and the RCs, and when we make A6 GA I'll release v3.0.0 of the cookbook with A6 installed by default, and all this legacy code removed.

Support process-agent configuration in datadog.yaml
Also remove process.conf for Agent 6

@conorbranagan conorbranagan force-pushed the conorbranagan:conor/process-agent branch from d474beb to 54bb3d2 Feb 13, 2018

@conorbranagan

This comment has been minimized.

Copy link
Member Author

conorbranagan commented Feb 13, 2018

@olivielpeau Should be good now, thanks

@olivielpeau
Copy link
Member

olivielpeau left a comment

looks good! Merging

Jaime approved but github dismissed the approval (!?)

@olivielpeau olivielpeau merged commit 884dd84 into DataDog:master Feb 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@truthbk truthbk added this to the 2.14.0 milestone Feb 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.