-
Notifications
You must be signed in to change notification settings - Fork 71
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
add jmxfetch telemetry in code #467
Changes from 8 commits
19cca1e
ee586ce
7c15096
b23544a
2619424
100e7ec
d5f23d4
a095a71
79f050f
a9ebf0f
4ab5e1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -28,6 +28,7 @@ | |||||
import java.util.Arrays; | ||||||
import java.util.Collection; | ||||||
import java.util.Collections; | ||||||
import java.util.HashMap; | ||||||
import java.util.Iterator; | ||||||
import java.util.List; | ||||||
import java.util.ListIterator; | ||||||
|
@@ -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) { | ||||||
log.info("Adding jmxfetch instance telemetry check"); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
final Instance instance = instantiate(getInstanceTelemetryInstanceConfig(), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
getInstanceTelemetryInitConfig(), "jmxfetch_telemetry_check", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
this.appConfig); | ||||||
newInstances.add(instance); | ||||||
} | ||||||
} | ||||||
|
||||||
final List<InstanceTask<Void>> instanceInitTasks = | ||||||
new ArrayList<>(newInstances.size()); | ||||||
for (Instance instance : newInstances) { | ||||||
|
@@ -945,6 +957,34 @@ public TaskStatusHandler invoke( | |||||
} | ||||||
} | ||||||
|
||||||
private Map<String,Object> getInstanceTelemetryInitConfig() { | ||||||
Map<String,Object> config = new HashMap<String,Object>(); | ||||||
config.put("is_jmx",true); | ||||||
return config; | ||||||
} | ||||||
|
||||||
private Map<String,Object> getInstanceTelemetryInstanceConfig() { | ||||||
Map<String,Object> config = new HashMap<String,Object>(); | ||||||
config.put("name","jmxfetch_telemetry_instance"); | ||||||
config.put("collect_default_jvm_metrics",true); | ||||||
config.put("new_gc_metrics",true); | ||||||
config.put("process_name_regex",".*org.datadog.jmxfetch.App.*"); | ||||||
|
||||||
List<Object> conf = new ArrayList<Object>(); | ||||||
Map<String,Object> confMap = new HashMap<String,Object>(); | ||||||
Map<String,Object> includeMap = new HashMap<String,Object>(); | ||||||
includeMap.put("domain","jmxfetch"); | ||||||
confMap.put("include", includeMap); | ||||||
conf.add(confMap); | ||||||
config.put("conf",conf); | ||||||
|
||||||
List<String> tags = new ArrayList<String>(); | ||||||
tags.add("jmx:fetch"); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||||||
config.put("tags", tags); | ||||||
|
||||||
return config; | ||||||
} | ||||||
|
||||||
static TaskStatusHandler processRecoveryResults( | ||||||
final Instance instance, | ||||||
final Future<Void> future, | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -122,6 +122,12 @@ public class AppConfig { | |||||
required = false) | ||||||
private boolean statsdTelemetry; | ||||||
|
||||||
@Parameter( | ||||||
names = {"--instance_telemetry", "-it"}, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
or maybe In the agent we simply use |
||||||
description = "Enable jmxfetch instance telemetry reporting", | ||||||
required = false) | ||||||
private boolean instanceTelemetry; | ||||||
|
||||||
@Parameter( | ||||||
names = {"--statsd_queue_size", "-sq"}, | ||||||
description = "Maximum number of unprocessed messages in the StatsD client queue.", | ||||||
|
@@ -411,6 +417,10 @@ public boolean getStatsdTelemetry() { | |||||
return statsdTelemetry; | ||||||
} | ||||||
|
||||||
public boolean getInstanceTelemetry() { | ||||||
return instanceTelemetry; | ||||||
} | ||||||
|
||||||
public int getStatsdQueueSize() { | ||||||
return statsdQueueSize; | ||||||
} | ||||||
|
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.
Lets fold this into the above
if
statement with a&&
to save some indentation levels.