Skip to content

add support for trusted fact tags and add info line for puppetserver …#662

Merged
albertvaka merged 10 commits intoDataDog:masterfrom
murdok5:trusted_facts
Nov 19, 2020
Merged

add support for trusted fact tags and add info line for puppetserver …#662
albertvaka merged 10 commits intoDataDog:masterfrom
murdok5:trusted_facts

Conversation

@murdok5
Copy link
Copy Markdown
Contributor

@murdok5 murdok5 commented Oct 21, 2020

…log notifications

What does this PR do?

Resolve issue where trusted facts were not working for report fact tags.

Motivation

Customer request based on experience/need.

Additional Notes

Added an unrelated info line to the report processor to log successful sends to Datadog API

Describe your test plan

Tested in a lab environment using Puppet Enterprise 2019.8.1 with current Datadog.

@murdok5 murdok5 requested a review from a team as a code owner October 21, 2020 23:37
@murdok5
Copy link
Copy Markdown
Contributor Author

murdok5 commented Oct 21, 2020

@albertvaka resolved the issues for trusted facts.

@albertvaka
Copy link
Copy Markdown
Contributor

Thanks a lot! We didn't know this was possible :) To be consistent with #658 I think we want to have a separate argument (eg: report_trusted_fact_tags) instead of merging the two hashes, but I will make that change on a separate PR. Your help is super appreciated 🙇

@murdok5
Copy link
Copy Markdown
Contributor Author

murdok5 commented Oct 22, 2020

@albertvaka either merged or separate works makes sense. The reason I merged into a single hash was user simplicity and trusted is a reserved name so there is not a chance of duplication.

@albertvaka
Copy link
Copy Markdown
Contributor

Yeah, I agree it's simpler like this, but since I initially added two separate arguments for facts_to_tags and trusted_facts_to_tags I think this should follow the same pattern with report_fact_tags and report_trusted_fact_tags.

@murdok5
Copy link
Copy Markdown
Contributor Author

murdok5 commented Oct 22, 2020

Yeah, I agree it's simpler like this, but since I initially added two separate arguments for facts_to_tags and trusted_facts_to_tags I think this should follow the same pattern with report_fact_tags and report_trusted_fact_tags.

Makes sense to stay consistent with that pattern.

@albertvaka
Copy link
Copy Markdown
Contributor

I pushed a commit to your branch that should do it, can you give it a quick look before I merge?

@murdok5
Copy link
Copy Markdown
Contributor Author

murdok5 commented Oct 24, 2020

I pushed a commit to your branch that should do it, can you give it a quick look before I merge?

Were you able to test this or you do want me to test in my lab?

@albertvaka
Copy link
Copy Markdown
Contributor

I've tested the Puppet side but not the Ruby script changes.

@murdok5 murdok5 requested a review from a team as a code owner November 5, 2020 17:54
Copy link
Copy Markdown
Contributor

@kayayarai kayayarai left a comment

Choose a reason for hiding this comment

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

Hey there Mike! A couple typo fixes and some formatting to help with documentation readability, in the README.md.

Comment thread README.md Outdated
5. (Optional) Enable tagging of reports with facts:

You can add tags to reports that are sent to Datadog as events. These tags can be sourced from Puppet facts for the given node the report is regarding. These should be 1:1 and not involve structured facts (hashes, arrays, etc.) to ensure readability. To enable tagging, set the parameter `datadog_agent::reports::report_fact_tags` to the array value of facts—for example `["virtual","trusted.extensions.pp_role","operatingsystem"]` results in three separate tags per report event.
You can add tags to reports that are sent to Datadog as events. These tags can be sourced from Puppet facts for the given node the report is regarding. These should be 1:1 and not involve structured facts (hashes, arrays, etc.) to ensure readability. To enable regular fact tagging, set the parameter `datadog_agent::reports::report_fact_tags` to the array value of facts—for example `["virtual","operatingsystem"]`. To enable trustd fact tagging, set the parameter `datadog_agent::reports::report_trusted_fact_tags` to the array value of facts—for example `["trusted.certname","trusted.extensions.pp_role","trusted hostname"]`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
You can add tags to reports that are sent to Datadog as events. These tags can be sourced from Puppet facts for the given node the report is regarding. These should be 1:1 and not involve structured facts (hashes, arrays, etc.) to ensure readability. To enable regular fact tagging, set the parameter `datadog_agent::reports::report_fact_tags` to the array value of facts—for example `["virtual","operatingsystem"]`. To enable trustd fact tagging, set the parameter `datadog_agent::reports::report_trusted_fact_tags` to the array value of facts—for example `["trusted.certname","trusted.extensions.pp_role","trusted hostname"]`.
You can add tags to reports that are sent to Datadog as events. These tags can be sourced from Puppet facts for the given node the report is regarding. These should be 1:1 and not involve structured facts (hashes, arrays, etc.) to ensure readability.
To enable regular fact tagging, set the parameter `datadog_agent::reports::report_fact_tags` to the array value of facts—for example `["virtual","operatingsystem"]`.
To enable trusted fact tagging, set the parameter `datadog_agent::reports::report_trusted_fact_tags` to the array value of facts—for example `["trusted.certname","trusted.extensions.pp_role","trusted.hostname"]`.

@murdok5
Copy link
Copy Markdown
Contributor Author

murdok5 commented Nov 5, 2020

@kayayarai @albertvaka sorry for delay but I did some testing and confirmed functionality. I updated the readme and some debugging if needed as well. I tried to differentiate between tags for Datadog agents and Report (event) tagging.

Comment thread README.md Outdated
Co-authored-by: Kari Halsted <12926135+kayayarai@users.noreply.github.com>
Comment thread lib/puppet/reports/datadog_reports.rb Outdated
Puppet.debug "Sending events for #{@msg_host} to Datadog"
facts_tags = REPORT_FACT_TAGS.map { |name| "#{name}:#{facts.dig(*name.split('.'))}" }
trusted_facts = (Puppet.lookup(:trusted_information) { Hash.new }).to_h
trusted_facts = { "trusted" => trusted_facts.to_h }
Copy link
Copy Markdown
Contributor

@albertvaka albertvaka Nov 6, 2020

Choose a reason for hiding this comment

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

To make sure report_trusted_fact_tags is consistent with trusted_facts_to_tags. When accessing the $trusted variable from puppet code, do you have to use the trusted.* prefix? (eg: $trusted['trusted. certname']). Since trusted_facts_to_tags just digs in $trusted.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

From the examples section in Puppet docs [1], it looks like the trusted prefix doesn't have to be used when accessing $trusted, so I'm going to remove it from this PR and merge it.

[1] https://puppet.com/docs/puppet/5.5/lang_facts_and_builtin_vars.html#examples

Copy link
Copy Markdown
Contributor

@kayayarai kayayarai left a comment

Choose a reason for hiding this comment

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

👍 for the documentation

So report_trusted_fact_tags is consistent wih trusted_facts_to_tags
Disable Style/EmptyLiteral since it's way less readable to do { {} }
@albertvaka albertvaka merged commit efbe0bf into DataDog:master Nov 19, 2020
cegeka-jenkins pushed a commit to cegeka/puppet-datadog_agent that referenced this pull request Feb 5, 2026
Adds report_trusted_fact_tags  which works like trusted_facts_to_tags for reports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants