Don't append "tag:" to tags collected for a node managed by Chef

[ovaistariq]

The Chef tags collection function appends "tag:" to the tags collected which doesn't make sense:

def get_node_tags(node)
  node.tags.map! { |tag| 'tag:' + tag }
end

Since these are already tags so it would be better to have them just collected as they are without appending "tag:". Majority of the times Chef tags are added to nodes to identify them specifically.

[miketheman]

Hi @ovaistariq! One of the reasons we prepend the tag: string to the tag value is to allow the Datadog's "group by" comparison on graphs.

For instance: avg:system.load.1{*} by {tag} would produce a graph in which the nodes with tags would be compared against nodes without tags (using N/A as their tag notation).

Can you elaborate on why the tag value in Datadog doesn't make sense?

[ovaistariq]

The reason I think it doesn't make sense is that it if there are Chef node tags such as id:value then they become tag:id:value. For example, for us service:mysql becomes tag:service:mysql in datadog. So if I need to search by service:mysql, I would instead have to search by tag:service:mysql.

[miketheman]

@ovaistariq That makes sense.

Since there's no deterministic way of setting Chef tags - they are arbitrary strings after all - would it make sense for the get_node_tags method to do a string match on the tag, and prepend tag: if no : is present?

This is potentially a breaking change to anyone that uses tags today inside Datadog with the tag: prefix, so it's something we should not do lightly.

[miketheman]

@ovaistariq Can you check out the linked code? If that makes sense, we can inspect to see how many users this might affect to see how disruptive it would be to make this change.

[ovaistariq]

@miketheman The code looks good to me as it covers both the cases (Y)

[ovaistariq]

I look forward to hearing what will happen to the fix after you have checked to see how many users actually depend on "tag:" being present in the tag name. Assuming the impact is marginal, would you have an idea as to when the next release "including the fix" would be.

[miketheman]

Update on this front.

Upon inspection, enough users are using the tag: prefix to make changing this outright a breaking change, and unexpected.

A likelier change will be to add a flag to active the new behavior, defaulting to false, and marking the existing one as deprecated for a few releases, as well as introducing a new one that can be activated on demand.

[ovaistariq]

@miketheman thanks for the update. Can I then expect a flag to be added to enable the new behavior.

[miketheman]

When we write it, yes. ⏰

On Fri, Oct 23, 2015, 21:06 Ovais Tariq notifications@github.com wrote:

@miketheman https://github.com/miketheman thanks for the update. Can I
then expect a flag to be added to enable the new behavior.

—
Reply to this email directly or view it on GitHub
#58 (comment)
.

[ovaistariq]

I meant to say is there an immediate plan or when should I expect to see this feature released?

[miketheman]

There's no immediate plan for this - it's something I've been working on in
my spare time.
If you've got some spare cycles and can put some code in, I'd be happy to
review it!

On Sat, Oct 24, 2015 at 4:55 PM, Ovais Tariq notifications@github.com
wrote:

I meant to say is there an immediate plan or when should I expect to see
this feature released?

—
Reply to this email directly or view it on GitHub
#58 (comment)
.

[andrewsf]

One of the reasons we prepend the tag: string to the tag value is to allow the Datadog's "group by" comparison on graphs.

That's exactly the problem. I want to group by values of the actual tags. I don't want to group by tuples of the tag name and value.

If Datadog is receiving tags like this:

tag:datacenter:east
tag:datacenter:west
tag:environment:prod
tag:environment:dev

Then what it sees is something called "tag" that has four values. I want to slice by "datacenter" and "environment". I would never want to slice by "tag" as that mixes up what are actually two different tags.

[miketheman]

@andrewsf Thanks for the notes! I think this was covered in #58 (comment).

I've been trying to determine a safe way to introduce a config flag to control the handler's tag behavior, and emitting a deprecation warning when the original method is used.

i've drafted a page here: https://github.com/DataDog/chef-handler-datadog/wiki/Legacy-Tags and would appreciate some read-through and confirming that I have your ideas in place before committing to a code path that may not be ideal.

I'm trying to allow for current users to continue to have the existing behaviors, but be warned about the upcoming change, and allow adoption of the new approach.

[miketheman]

The change to support this has been released in the Handler, and the next step is to release the Chef Cookbook changes to support the user selection of prefix.

Closing, thanks!