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

Cluster-checks: patch configurations on schedule #2862

Merged
merged 1 commit into from
Jan 23, 2019

Conversation

xvello
Copy link
Contributor

@xvello xvello commented Jan 8, 2019

What does this PR do?

When configurations enter the clusterchecks dispatcher from the AD, make the following changes:

  • empty the ADIdentifiers array, to avoid node-agents detecting them as templates
  • clear the ClusterCheck boolean
  • add the empty_default_hostname option to all instances, so metrics are submitted with no hostname
  • optionaly inject the cluster_name tag in all instances

Doing the modifications here make digests different between the DCA's AD and the clusterchecks logic, but ensure they will be identical between the DCA and the node-agnets, which will be more useful for debugging.

This step of the pipeline has been chosen because this is the earliest point in the pipeline all necessary information is available to batch all required changes in one operation.

@codecov-io
Copy link

Codecov Report

Merging #2862 into master will decrease coverage by <.01%.
The diff coverage is 64.28%.

@@            Coverage Diff            @@
##           master   #2862      +/-   ##
=========================================
- Coverage    57.4%   57.4%   -0.01%     
=========================================
  Files         392     392              
  Lines       25616   25662      +46     
=========================================
+ Hits        14705   14730      +25     
- Misses       9943    9957      +14     
- Partials      968     975       +7
Impacted Files Coverage Δ
pkg/config/config.go 83.4% <100%> (+0.03%) ⬆️
pkg/autodiscovery/integration/config.go 55.24% <57.14%> (+0.2%) ⬆️
...g/clusteragent/clusterchecks/dispatcher_configs.go 85.05% <61.9%> (-7.37%) ⬇️
pkg/clusteragent/clusterchecks/dispatcher_main.go 79.16% <70%> (-6.32%) ⬇️
pkg/logs/input/listener/tailer.go 86% <0%> (-4%) ⬇️
pkg/logs/client/connection_manager.go 44.82% <0%> (-3.45%) ⬇️
pkg/logs/auditor/auditor.go 71.83% <0%> (+0.7%) ⬆️
pkg/forwarder/worker.go 93.75% <0%> (+1.25%) ⬆️
pkg/forwarder/domain_forwarder.go 92.91% <0%> (+1.57%) ⬆️

Copy link
Contributor

@mfpierre mfpierre left a comment

Choose a reason for hiding this comment

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

LGTM overall, just one question

func (d *dispatcher) patchConfiguration(in integration.Config) (integration.Config, error) {
out := in
out.ADIdentifiers = nil
out.ClusterCheck = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we do this? Don't we want to be able to identify these? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The check collector ignores configurations with that flag. This is needed to avoid having them run in the DCA's collector.

The patching logic is taking a cluster-check configuration and outputting a "classic" check configuration ready to run on a node.

We are able to identify them on the node-agent's configcheck via their source (the clusterchecks config provider), see an example output in https://docs-staging.datadoghq.com/xvello/cchecks/agent/autodiscovery/clusterchecks/#autodiscovery-in-the-node-based-agent

@xvello xvello merged commit 2de18dd into master Jan 23, 2019
@xvello xvello deleted the xvello/ccheck-patch-config branch January 23, 2019 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants