-
Notifications
You must be signed in to change notification settings - Fork 70
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
[service-checks] adds tags from configuration file to service checks #96
Conversation
I had to make small changes to some of the tests. With this change they were expecting the wrong number of tags. |
@@ -83,7 +83,7 @@ public void testServiceCheckWarning() throws Exception { | |||
assertEquals(Reporter.formatServiceCheckPrefix("too_many_metrics"), scName); | |||
// We should have an OK service check status when too many metrics are getting sent | |||
assertEquals(Status.STATUS_OK, scStatus); | |||
assertEquals(scTags.length, 1); | |||
assertEquals(scTags.length, 3); | |||
assertTrue(Arrays.asList(scTags).contains("instance:jmx_test_instance")); |
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.
Can we tests the value of the 2 extra tags too ?
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.
Yes, I can do that. In fact it was a good idea. The way I was adding tags made these tags:
env: stage
newTag: test
be output as just stage
and test
I've added another commit that will change that to env:stage
newTag:test
which is what I assume we want.
It looks good to me. |
Nice, let's merge it. |
**Changes** * [BUGFIX] Report properly beans with ':' in the name. See [#90][], * [#91][], [#95][] (Thanks [@Bluestix][]) * [BUGFIX] Sanitize metric names and tags, i.e. remove illegal * characters. See [#89][] * [BUGFIX] Support `javax.management.Attribute` attribute types. See * [#92][] (Thanks [@nwillems][]) * [FEATURE] Add user tags to service checks. See [#96][] * [FEATURE] Allow group name substitutions in attribute/alias * parameters. See [#94][], [#97][] (Thanks [@alz][]) <!--- The following link definition list is generated by PimpMyChangelog ---> [#89]: DataDog/jmxfetch#89 [#90]: DataDog/jmxfetch#90 [#91]: DataDog/jmxfetch#91 [#92]: DataDog/jmxfetch#92 [#94]: DataDog/jmxfetch#94 [#95]: DataDog/jmxfetch#95 [#96]: DataDog/jmxfetch#96 [#97]: DataDog/jmxfetch#97 [@alz]: https://github.com/alz [@Bluestix]: https://github.com/bluestix [@nwillems]: https://github.com/nwillems
**Changes** * [BUGFIX] Report properly beans with ':' in the name. See [#90][], * [#91][], [#95][] (Thanks [@Bluestix][]) * [BUGFIX] Sanitize metric names and tags, i.e. remove illegal * characters. See [#89][] * [BUGFIX] Support `javax.management.Attribute` attribute types. See * [#92][] (Thanks [@nwillems][]) * [FEATURE] Add user tags to service checks. See [#96][] * [FEATURE] Allow group name substitutions in attribute/alias * parameters. See [#94][], [#97][] (Thanks [@alz][]) <!--- The following link definition list is generated by PimpMyChangelog ---> [#89]: DataDog/jmxfetch#89 [#90]: DataDog/jmxfetch#90 [#91]: DataDog/jmxfetch#91 [#92]: DataDog/jmxfetch#92 [#94]: DataDog/jmxfetch#94 [#95]: DataDog/jmxfetch#95 [#96]: DataDog/jmxfetch#96 [#97]: DataDog/jmxfetch#97 [@alz]: https://github.com/alz [@Bluestix]: https://github.com/bluestix [@nwillems]: https://github.com/nwillems
Currently, only the scope and instance tags are available on service checks.
This PR will make it so that, in addition to those tags, all the tags defined in the configuration file will be available to service checks, making it much easier to scope an integration monitor to a set of hosts.