datadog-agent icon indicating copy to clipboard operation
datadog-agent copied to clipboard

Poll agent integration configuration files for changes

Open vboulineau opened this issue 1 year ago • 3 comments

What does this PR do?

Original contribution: PR#11162.

Allows the file AD provider to reload check configurations periodically

Motivation

Allows to inject dynamically checks to the Agent or Cluster Agent Can be useful in non-Kubernetes environments where Cluster Agent is not available to schedule service checks.

Additional Notes

The feature is technically available in Agent and Cluster Agent, yet usage of advanced_ad_identifiers should be preferred in the Cluster Agent.

Possible Drawbacks / Trade-offs

Logs config do not always support being unscheduled. Flagging this limitation in the changelog and in the config.

Describe how to test/QA your changes

Deploy the Agent in any environment with DD_AUTOCONF_CONFIG_FILES_POLL set to true. Add/remove configuration files in /etc/datadog-agent/conf.d, observe that the Agent loads and unloads check configs properly.

Reviewer's Checklist

  • [x] If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • [x] Use the major_change label if your change either has a major impact on the code base, is impacting multiple teams or is changing important well-established internals of the Agent. This label will be use during QA to make sure each team pay extra attention to the changed behavior. For any customer facing change use a releasenote.
  • [x] A release note has been added or the changelog/no-changelog label has been applied.
  • [x] Changed code has automated tests for its functionality.
  • [x] Adequate QA/testing plan information is provided if the qa/skip-qa label is not applied.
  • [x] At least one team/.. label has been applied, indicating the team(s) that should QA this change.
  • [x] If applicable, docs team has been notified or an issue has been opened on the documentation repo.
  • [x] If applicable, the need-change/operator and need-change/helm labels have been applied.
  • [x] If applicable, the k8s/<min-version> label, indicating the lowest Kubernetes version compatible with this feature.
  • [x] If applicable, the config template has been updated.

vboulineau avatar Oct 06 '22 13:10 vboulineau

Would it be possible to add tests for this?

We don't need to add tests for two reasons:

  • Being a polling or non-polling provider is not specific to this provider.
  • The code is already tested in itself, we're just changing the cache expiration time.

vboulineau avatar Oct 10 '22 12:10 vboulineau

We still don't test the feature as a whole: that configuration options actually enable the feature, that the supposed interaction between the polling logic and the cache expiration actually leads to checks being updated. Wouldn't it make sense to test this?

vickenty avatar Oct 11 '22 09:10 vickenty

@vickenty I've added a test that validates the interaction with Config but we cannot really test the registration part.

@djmitche I've change the NOTICE into WARNING about logs config. I believe experimental or beta is not really it, the checks part works well, and is, perhaps, the most important one.

vboulineau avatar Oct 13 '22 12:10 vboulineau