[zk] Modified sc_tags in zk.py to include user-specified tags.#3078
Merged
truthbk merged 1 commit intoDataDog:masterfrom Jan 30, 2017
Merged
[zk] Modified sc_tags in zk.py to include user-specified tags.#3078truthbk merged 1 commit intoDataDog:masterfrom
truthbk merged 1 commit intoDataDog:masterfrom
Conversation
hkaj
approved these changes
Jan 2, 2017
hkaj
reviewed
Jan 2, 2017
checks.d/zk.py
Outdated
| tags = instance.get('tags', []) | ||
| cx_args = (host, port, timeout) | ||
| sc_tags = ["host:{0}".format(host), "port:{0}".format(port)] | ||
| sc_tags = ["host:{0}".format(host), "port:{0}".format(port)] + tags |
Member
There was a problem hiding this comment.
actually a nitpick here: could you list(set(...)) that please? Just in case...
5da094b to
42ce1c8
Compare
Member
|
Thanks @arzarif, very much appreciated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
The manner in which tags are applied to service checks and metric checks depends on 2 separate variables. This change modifies the variable that encapsulates tags relevant to service checks (
sc_tags). Currently for service checks, this variable applies only the host and port information provided in the instance configuration. This change will allow user-defined tags to similarly be applied to service checks (as is the case with metric checks).Motivation
Several of the core integrations handle tags differently for the purposes of metric checks vs. service checks. It's unclear what motivated the inconsistent handling of tags in these contexts. When users are using tags to template alerts, this presents an unnecessary limitation that impacts the ability to template service check-based alerts.
While this PR addresses this inconsistency in one of these integrations (Zookeeper), there should be an effort to normalize the handling of tags in different contexts across natively-supported integrations.