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

add jmxfetch telemetry in code #467

Merged
merged 11 commits into from
Aug 11, 2023
40 changes: 40 additions & 0 deletions src/main/java/org/datadog/jmxfetch/App.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Copy link
Contributor

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.

log.info("Adding jmxfetch instance telemetry check");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log.info("Adding jmxfetch instance telemetry check");
log.info("Adding jmxfetch telemetry check");

final Instance instance = instantiate(getInstanceTelemetryInstanceConfig(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
final Instance instance = instantiate(getInstanceTelemetryInstanceConfig(),
final Instance instance = instantiate(getTelemetryInstanceConfig(),

getInstanceTelemetryInitConfig(), "jmxfetch_telemetry_check",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
getInstanceTelemetryInitConfig(), "jmxfetch_telemetry_check",
getTelemetryInitConfig(), "jmxfetch_telemetry_check",

this.appConfig);
newInstances.add(instance);
}
}

final List<InstanceTask<Void>> instanceInitTasks =
new ArrayList<>(newInstances.size());
for (Instance instance : newInstances) {
Expand Down Expand Up @@ -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");
Copy link
Contributor

Choose a reason for hiding this comment

The 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 tags.add("jmxfetch_version:0.47.10").

config.put("tags", tags);

return config;
}

static TaskStatusHandler processRecoveryResults(
final Instance instance,
final Future<Void> future,
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/org/datadog/jmxfetch/AppConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ public class AppConfig {
required = false)
private boolean statsdTelemetry;

@Parameter(
names = {"--instance_telemetry", "-it"},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

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.",
Expand Down Expand Up @@ -411,6 +417,10 @@ public boolean getStatsdTelemetry() {
return statsdTelemetry;
}

public boolean getInstanceTelemetry() {
return instanceTelemetry;
}

public int getStatsdQueueSize() {
return statsdQueueSize;
}
Expand Down