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

Handle metadata collection flag on MetadataManager #6077

Closed

Conversation

florimondmanca
Copy link
Contributor

@florimondmanca florimondmanca commented Mar 17, 2020

What does this PR do?

Deprecate AgentCheck.is_metadata_collection_enabled() in favor of filtering submissions directly on MetadataManager.

TODO: actually this doesn't prevent integrations from doing unnecessary processing to retrieve metadata (eg fetching the version), which is probably why we added is_metadata_collection_enabled() in the first place. Hmm…

Motivation

  • Remove the need to always check for is_metadata_collection_enabled() in integrations, reducing boilerplate and potential for errors.
  • Immediately add support for honoring enable_metadata_collection Agent flag to all integrations. (Otherwise we're going to have and add an if statement to all our integrations, eg see Allow disabling metadata collection in fluentd #6061.)

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@florimondmanca florimondmanca deleted the florimondmanca/metadata-manager-enabled branch March 17, 2020 16:00
@florimondmanca
Copy link
Contributor Author

Closed, as integrations typically need to make processing to retrieve the version, and we'd want to prevent that processing from happening too, which this doesn't solve.

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.

1 participant