[couchbase] Modified service_check_tags in couchbase.py to include user-specified tags. #3079
Conversation
This file contains 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
hkaj
changed the title
hkaj
requested changes
Jan 2, 2017
| if service_check_tags is None: | ||
| service_check_tags = [] | ||
| else: | ||
| service_check_tags = list(set(service_check_tags)) |
There was a problem hiding this comment.
i think you can replace all that with service_check_tags = list(set(instance.get('tags') or []))
There was a problem hiding this comment.
@hkaj I made the changes in this way to keep service tag collection consistent with how tags are collected elsewhere in this check. See:
gmmeyer
approved these changes
Feb 7, 2017
degemer
added a commit
that referenced
this pull request
Feb 7, 2017
* master: (67 commits) [network] dont combine connection states (#3158) renames function in line with other checks [couchbase] Modified service_check_tags in couchbase.py to include user-specified tags. (#3079) [docker] fix image tag extraction Fix tests, refactor how we collect container and volume states test_docker_daemon.py: fix syntax errors Beginning work on docker_daemon tests. Add 5 opt-in checks to docker_daemon add mention of office hours (#3171) updates psycopg2 to 2.6.2 (#3170) [trace-agent] adding output of 'trace-agent -info' (#3164) [riak] Change default value in configuration example to match default value from the code move setting parameter to instance level riak security support [dns_check][ci] Bring back assertions on metrics (#3162) [powerdns_recursor] adds support for v4. (#3166) [tcp_check] Add custom tags to response_time gauge catch can't connect instead of failing on nodata found (#3127) [php-fpm] add http_host tag [dns_check] Document NXDOMAIN usage in yaml example file ...
gmmeyer
changed the title
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 (
service_check_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 (Couchbase), there should be an effort to normalize the handling of tags in different contexts across natively-supported integrations.