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

Add a service_blacklist configuration to ensure certain services never have metrics collected #1527

Closed

Conversation

TylerLubeck
Copy link
Contributor

…r have metrics collected.

Closes #1186

What does this PR do?

Addsa service_blacklist configuration to ensure certain services never have metrics collected

Motivation

I was on a role making consul changes and saw #1186 so I tackled it

Versioning

  • Updated CHANGELOG.md. Please use Unreleased as the date in the title

@masci masci added the community label May 9, 2018
@ofek
Copy link
Contributor

ofek commented May 10, 2018

@TylerLubeck Hello there! Would you mind rebasing and using the new filter utility that was introduced in #1510?

I really appreciate it 🙇

print(services, service_blacklist)
services = {
s: services[s] for s in services if s not in service_blacklist
}
Copy link
Contributor

Choose a reason for hiding this comment

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

For blacklisting, can you use this function that was just added in the base check? https://github.com/DataDog/integrations-core/blob/master/datadog_checks_base/datadog_checks/utils/common.py#L14 We're trying to consolidate the way we do this across the whole codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can, but I think either there's a bug in that function or it doesn't do quite what we would need here:

https://github.com/DataDog/integrations-core/blob/master/datadog_checks_base/datadog_checks/utils/common.py#L47

I think that in the case that there's a blacklist, then the items have the blacklisted items removed but they're not restricted to just the whitelisted items.

@masci masci self-assigned this May 10, 2018
@TylerLubeck TylerLubeck force-pushed the consul_service_blacklist branch from fcb9389 to e85eea0 Compare May 15, 2018 23:55
@TylerLubeck TylerLubeck force-pushed the consul_service_blacklist branch from e85eea0 to a92b30a Compare May 15, 2018 23:57
@TylerLubeck
Copy link
Contributor Author

Copying out of a closed comment thread:

I can use the pattern_filter function, but I think either there's a bug in that function or it doesn't do quite what we would need here:

https://github.com/DataDog/integrations-core/blob/master/datadog_checks_base/datadog_checks/utils/common.py#L47

I think that in the case that there's a blacklist, then the items have the blacklisted items removed but they're not restricted to just the whitelisted items.

I've pushed up the code change to start using the function anyway, if only for the sake of not neglecting this for a week

@masci
Copy link
Contributor

masci commented May 19, 2018

@ofek can you chime in?

@ofek
Copy link
Contributor

ofek commented May 22, 2018

@TylerLubeck You can chain the function one type of list at a time.

That is not a bug btw, but rather some tricky membership logic: the only way to use a WL/BL at the same time without either being completely ignored is by applying the BL without members of the WL.

@stale
Copy link

stale bot commented Jun 25, 2018

This issue has been automatically marked as stale because it has not had activity in the last 30 days. Note that the issue will not be automatically closed, but this notification will remind us to investigate why there's been inactivity. Thank you for participating in the Datadog open source community.

@ofek
Copy link
Contributor

ofek commented Nov 23, 2018

Resolved by #2174

@ofek ofek closed this Nov 23, 2018
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