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

Adding support for list of datadog tags in v6 configuration file #557

Merged
merged 3 commits into from Feb 26, 2019

Conversation

@skarlupka
Copy link
Contributor

commented Aug 22, 2018

Fixes #556

@skarlupka skarlupka force-pushed the skarlupka:hskarlupka/v6-tags branch from db06e76 to 71ab1cf Aug 22, 2018
@olivielpeau olivielpeau added this to the 2.17.0 milestone Oct 23, 2018
@tschroeder-zendesk

This comment has been minimized.

Copy link

commented Nov 26, 2018

Any estimate on when this might get merged?

@remeh

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2019

Hey @skarlupka , thank you for your contribution!
I've investigated this and indeed we have a different behavior between the Agent 5 and Agent 6 while using a list of tags.

Before merging your pull-request I would like to add some commits to have the exact same behavior between Agent 5 and Agent 6 (the list of tags injected in the Agent 5 configuration is working but not 100% correct).

Could I ask you to tick the box letting me to push commits in your PR? Thank you.
Edit: don't mind the last sentence, it looks like I can already push to your branch.

@skarlupka

This comment has been minimized.

Copy link
Contributor Author

commented Feb 21, 2019

Hello @remeh!

The "Allow edits from maintainers" is checked in the PR. Let me know if there is anything else I need to do.

They were previously written as their string representation, meaning
an array containg "custom:tag1" and "custom:tag2" would have created the
tag `["custom:tag1"` and `"custom:tag2"]`. It was working correctly because
the Datadog servers were cleaning those extra chars.
@remeh

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2019

Thank you @skarlupka

I've added a condition to correctly handle array of tags in the Agent 5 configuration + I've slighty changed your PR to stay with respond_to tests to be consistent with the rest of the repo.

Copy link
Member

left a comment

LGTM 👍

Could you also update the attribute documentation here?

# This can be a string of comma separated tags or a hash in this format:
# default['datadog']['tags'] = { 'datacenter' => 'us-east' }
# Thie above outputs a string: 'datacenter:us-east'
# When using the Datadog Chef Handler, tags are set on the node with preset prefixes:
# `env:node.chef_environment`, `role:node.node.run_list.role`, `tag:somecheftag`
default['datadog']['tags'] = ''

As we discussed offline a list on node['datadog']['tags'] wasn't supported officially by the cookbook, but did work on Agent 5, even though it wasn't by design: the INI format of Agent 5 doesn't support non-scalar values like lists, so for a config like:

tags: ["foo:bar", "bar:baz"]

Agent 5 sends the tags ["foo:bar" and "bar:baz"], but the Datadog backend then cleans up the characters that aren't allowed in tags so that foo:bar and bar:baz are indeed applied as tags to the host.

@remeh remeh modified the milestones: 2.17.0, 3.0 Feb 26, 2019
@remeh

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2019

Thanks again for your contribution @skarlupka!

@tschroeder-zendesk I'm merging this to master now and it'll be part of release 2.17.0

@remeh remeh merged commit 5801492 into DataDog:master Feb 26, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
ci/circleci: test Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.