-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Sanitize external host tags #3792
Conversation
@@ -152,7 +152,7 @@ def check(self, instance): | |||
self.service_check(SERVICE_CHECK_NAME, AgentCheck.OK, tags=service_check_tags) | |||
|
|||
if set_external_tags: | |||
set_external_tags(self.get_external_host_tags()) | |||
self.set_external_tags(self.get_external_host_tags()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove the import for set_external_tags
at the top ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No b/c we need a way to tell if on A5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's do
if hasattr(self, "set_external_tags"):
then, and remove the import at the top
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or just remove completely that if
. We have pinned integrations in A5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
➕
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. It seems we have the issue in openstack and openstack_controller, too.
Codecov Report
@@ Coverage Diff @@
## master #3792 +/- ##
=========================================
+ Coverage 86.74% 88.9% +2.15%
=========================================
Files 730 38 -692
Lines 37649 8112 -29537
Branches 4467 520 -3947
=========================================
- Hits 32660 7212 -25448
+ Misses 3799 755 -3044
+ Partials 1190 145 -1045 |
Motivation
Handle bytes or unicode