add jmxfetch telemetry in code #467
Conversation
There was a problem hiding this comment.
Probably the last set of changes I'll request.
To re-iterate what we discussed, lets make the config option that turns this on and off be generic -- "JMXFetch Telemetry". Since the domain monitored by the telemetry instance is not specific to instance telemetry, the config option to turn it on and off should not be either.
| @@ -897,6 +898,17 @@ public void init(final boolean forceNewConnection) { | |||
| } | |||
| } | |||
|
|
|||
| if (appConfig.getInstanceTelemetry()) { | |||
| // If we have at least one check add jmxfetch_telemetry check | |||
| if (newInstances.size() >= 1) { | |||
There was a problem hiding this comment.
Lets fold this into the above if statement with a && to save some indentation levels.
| if (appConfig.getInstanceTelemetry()) { | ||
| // If we have at least one check add jmxfetch_telemetry check | ||
| if (newInstances.size() >= 1) { | ||
| log.info("Adding jmxfetch instance telemetry check"); |
There was a problem hiding this comment.
| log.info("Adding jmxfetch instance telemetry check"); | |
| log.info("Adding jmxfetch telemetry check"); |
| // If we have at least one check add jmxfetch_telemetry check | ||
| if (newInstances.size() >= 1) { | ||
| log.info("Adding jmxfetch instance telemetry check"); | ||
| final Instance instance = instantiate(getInstanceTelemetryInstanceConfig(), |
There was a problem hiding this comment.
| final Instance instance = instantiate(getInstanceTelemetryInstanceConfig(), | |
| final Instance instance = instantiate(getTelemetryInstanceConfig(), |
| if (newInstances.size() >= 1) { | ||
| log.info("Adding jmxfetch instance telemetry check"); | ||
| final Instance instance = instantiate(getInstanceTelemetryInstanceConfig(), | ||
| getInstanceTelemetryInitConfig(), "jmxfetch_telemetry_check", |
There was a problem hiding this comment.
| getInstanceTelemetryInitConfig(), "jmxfetch_telemetry_check", | |
| getTelemetryInitConfig(), "jmxfetch_telemetry_check", |
| @@ -122,6 +122,12 @@ public class AppConfig { | |||
| required = false) | |||
| private boolean statsdTelemetry; | |||
|
|
|||
| @Parameter( | |||
| names = {"--instance_telemetry", "-it"}, | |||
There was a problem hiding this comment.
| names = {"--instance_telemetry", "-it"}, | |
| names = {"--jmxfetch_telemetry", "-it"}, |
or maybe internal_telemetry? or maybe just telemetry?
In the agent we simply use telemetry, so maybe that works for us here.
cmetz100
changed the title
| config.put("conf",conf); | ||
|
|
||
| List<String> tags = new ArrayList<String>(); | ||
| tags.add("jmx:fetch"); |
There was a problem hiding this comment.
For this PR, lets not put any tags, just keep it as an empty list. This tag isn't particularly useful, I think this originated from some testing code.
In a follow-up PR, I'd like you to work on adding a tag that reflects the current version of JMXFetch. So tags.add("jmxfetch_version:0.47.10").
|
Please add a changelog entry and address the one last comment about tags and this is good to go! |
This pr is part of the multi stage process to increase observability within jmxfetch. With this pr I register java management beans that expose additional information about the types and amounts of data collected by each jmxfetch instance. In this pr I configure the actual check to collect both the beans I created in the other pr, in addition to default jvm metrics, and any other beans we may be interested in the future. It is possible to configure this check just as you would with any other having it be mounted in the conf.d directory or added as an autodiscovery label. However, this results in jmxfetch being run on every agent instance regardless if the agent is actually monitoring any other jvm. With this fact in mind we decided to add the check in the code of jmxfetch which can be enabled from a config option passed through the agent (jmx_telemetry_enabled). This allows us to dynamically deploy this check as we pick up other jmx checks and delete it when it is the only remaining check left. Doing this not only prevent us from starting unnecessary jmxfetch processes but also allows us to have more control over when this check is active.