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

[service-checks] adds tags from configuration file to service checks #96

Merged
merged 2 commits into from
Apr 28, 2016

Conversation

gmmeyer
Copy link
Contributor

@gmmeyer gmmeyer commented Apr 27, 2016

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.

@gmmeyer
Copy link
Contributor Author

gmmeyer commented Apr 27, 2016

I had to make small changes to some of the tests. With this change they were expecting the wrong number of tags.

@yannmh yannmh self-assigned this Apr 28, 2016
@@ -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"));
Copy link
Member

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 ?

Copy link
Contributor Author

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.

@yannmh
Copy link
Member

yannmh commented Apr 28, 2016

It looks good to me.
I added a small nitpick, although it is ready to be merged.

@yannmh yannmh added this to the 0.11.0 milestone Apr 28, 2016
@yannmh yannmh merged commit 8689358 into master Apr 28, 2016
@yannmh yannmh deleted the greg/service-check-tags branch April 28, 2016 22:37
@yannmh
Copy link
Member

yannmh commented Apr 28, 2016

Nice, let's merge it.

yannmh added a commit to DataDog/dd-agent that referenced this pull request May 13, 2016
**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
yannmh added a commit to DataDog/dd-agent that referenced this pull request May 13, 2016
**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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants