Skip to content

bdu/testability-of-default-metrics #568

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.

Sign up for GitHub

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

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

bdu/testability-of-default-metrics #568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5bde435
fix: Use BufferedInputStream to read config files
bric3 Jul 8, 2025
a017117
fix: File Descriptor leak
bric3 Jul 8, 2025
5213788
chore: Rewrite temp configuration test code
bric3 Jul 8, 2025
4290a96
chore: Test ZGC memory pools
bric3 Jul 8, 2025
032c64b
chore: Checkstyle nitpicks
bric3 Jul 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/main/java/org/datadog/jmxfetch/App.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.datadog.jmxfetch.util.LogLevel;
import org.datadog.jmxfetch.util.ServiceCheckHelper;

import java.io.BufferedInputStream;
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileInputStream;
Expand Down Expand Up @@ -764,7 +765,8 @@ private void loadFileConfigs(final AppConfig config, final Map<String, YamlParse
final String name = file.getName().replace(".yaml", "");
final String yamlPath = file.getAbsolutePath();
log.info("Reading {}", yamlPath);
try (FileInputStream yamlInputStream = new FileInputStream(yamlPath)) {
try (BufferedInputStream yamlInputStream =
new BufferedInputStream(new FileInputStream(yamlPath))) {
configs.put(name, new YamlParser(yamlInputStream));
} catch (FileNotFoundException e) {
log.warn("Cannot find " + yamlPath);
Expand Down
36 changes: 19 additions & 17 deletions src/main/java/org/datadog/jmxfetch/Instance.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.snakeyaml.engine.v2.api.Load;
import org.snakeyaml.engine.v2.api.LoadSettings;

import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
Expand Down Expand Up @@ -243,7 +244,13 @@ public Instance(

Boolean collectDefaultJvmMetrics = (Boolean) instanceMap.get("collect_default_jvm_metrics");
if (collectDefaultJvmMetrics == null || collectDefaultJvmMetrics) {
loadDefaultConfig("default-jmx-metrics.yaml");
// override the test definitions
Object value = instanceMap.get("default-jmx-metrics-definitions");
if (value instanceof String) {
loadDefaultConfig((String) value);
} else {
loadDefaultConfig("default-jmx-metrics.yaml");
}
loadDefaultConfig(gcMetricConfig);
} else {
log.info("collect_default_jvm_metrics is false - not collecting default JVM metrics");
Expand Down Expand Up @@ -295,11 +302,14 @@ public static boolean isDirectInstance(Map<String, Object> configInstance) {
}

private void loadDefaultConfig(String configResourcePath) {
InputStream is = this.getClass().getResourceAsStream(configResourcePath);
List<Map<String, Object>> defaultConf = (List<Map<String, Object>>)
YAML.get().loadFromInputStream(is);
for (Map<String, Object> conf : defaultConf) {
configurationList.add(new Configuration(conf));
try (InputStream is = this.getClass().getResourceAsStream(configResourcePath)) {
List<Map<String, Object>> defaultConf;
defaultConf = (List<Map<String, Object>>) YAML.get().loadFromInputStream(is);
for (Map<String, Object> conf : defaultConf) {
configurationList.add(new Configuration(conf));
}
} catch (IOException e) {
log.warn("Cannot parse internal default config file {}", configResourcePath, e);
}
}

Expand All @@ -315,10 +325,10 @@ static void loadMetricConfigFiles(
+ "migrate to using standard agent config files in the conf.d directory.");
for (String fileName : metricConfigFiles) {
String yamlPath = new File(fileName).getAbsolutePath();
FileInputStream yamlInputStream = null;

log.info("Reading metric config file " + yamlPath);
try {
yamlInputStream = new FileInputStream(yamlPath);
try (BufferedInputStream yamlInputStream =
new BufferedInputStream(new FileInputStream(yamlPath))) {
List<Map<String, Object>> confs =
(List<Map<String, Object>>)
YAML.get().loadFromInputStream(yamlInputStream);
Expand All @@ -329,14 +339,6 @@ static void loadMetricConfigFiles(
log.warn("Cannot find metric config file " + yamlPath);
} catch (Exception e) {
log.warn("Cannot parse yaml file " + yamlPath, e);
} finally {
if (yamlInputStream != null) {
try {
yamlInputStream.close();
} catch (IOException e) {
// ignore
}
}
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions src/main/resources/org/datadog/jmxfetch/default-jmx-metrics.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@
Usage.used:
alias: jvm.gc.eden_size
metric_type: gauge
# Generational ZGC
- include:
domain: java.lang
type: MemoryPool
Expand Down Expand Up @@ -197,6 +198,16 @@
Usage.used:
alias: jvm.gc.old_gen_size
metric_type: gauge
# Non-Generational ZGC
- include:
domain: java.lang
type: MemoryPool
name: ZHeap
attribute:
Usage.used:
alias: jvm.gc.old_gen_size
metric_type: gauge
# Generational ZGC
- include:
domain: java.lang
type: MemoryPool
Expand Down
89 changes: 39 additions & 50 deletions src/test/java/org/datadog/jmxfetch/TestCommon.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,9 @@

import com.beust.jcommander.JCommander;

import java.io.BufferedWriter;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileWriter;
import java.io.IOException;
import java.io.*;
import java.lang.management.ManagementFactory;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
Expand All @@ -33,30 +30,8 @@
import org.datadog.jmxfetch.util.CustomLogger;
import org.datadog.jmxfetch.util.MetricsAssert;
import org.datadog.jmxfetch.util.LogLevel;

final class ConfigUtil {
public static Path writeConfigYamlToTemp(String content, String yamlName) throws IOException {
Path tempDirectory = Files.createTempDirectory("temp-dir");
// Create a temporary file within the temporary directory
Path tempFile = Files.createTempFile(tempDirectory, yamlName, ".yaml");

// Write the contents of the file to the temporary file
BufferedWriter writer = new BufferedWriter(new FileWriter(tempFile.toFile()));
writer.write(content);
writer.close();

return tempFile;
}

public static String concatWithNewlines(String... lines) {
StringBuilder sb = new StringBuilder();
for (String line : lines) {
sb.append(line).append(System.lineSeparator());
}
return sb.toString();
}
}

import org.junit.Rule;
import org.junit.rules.TemporaryFolder;

public class TestCommon {
AppConfig appConfig = spy(AppConfig.builder().build());
Expand All @@ -66,9 +41,12 @@ public class TestCommon {
List<Map<String, Object>> metrics = new ArrayList<>();
List<Map<String, Object>> serviceChecks;

@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();

/** Setup logger. */
@BeforeClass
public static void init() throws Exception {
public static void init() {
String level = System.getProperty("tests.log_level");
if (level == null) {
level = "ALL";
Expand Down Expand Up @@ -132,10 +110,7 @@ protected void initApplication(String yamlFileName, String autoDiscoveryPipeFile
// We initialize the main app that will collect these metrics using JMX
String confdDirectory =
Thread.currentThread().getContextClassLoader().getResource(yamlFileName).getPath();
confdDirectory =
new String(
confdDirectory.substring(
0, confdDirectory.length() - yamlFileName.length()));
confdDirectory = confdDirectory.substring(0, confdDirectory.length() - yamlFileName.length());
List<String> params = new ArrayList<String>();
boolean sdEnabled = (autoDiscoveryPipeFile.length() > 0);
params.add("--reporter");
Expand All @@ -154,7 +129,7 @@ protected void initApplication(String yamlFileName, String autoDiscoveryPipeFile
params.add(5, "/foo"); // could be anything we're stubbing it out
params.add(6, "--sd_enabled");
}
new JCommander(appConfig, params.toArray(new String[params.size()]));
new JCommander(appConfig, params.toArray(new String[0]));

if (sdEnabled) {
String autoDiscoveryPipe =
Expand All @@ -179,7 +154,7 @@ protected void initApplication(String yamlFileName, String autoDiscoveryPipeFile
app.init(false);
}

protected void initApplication(String yamlFileName) throws FileNotFoundException, IOException {
protected void initApplication(String yamlFileName) throws IOException {
initApplication(yamlFileName, "");
}

Expand All @@ -188,31 +163,45 @@ protected void initApplication(String yamlFileName) throws FileNotFoundException
* The configuration can be specified as a yaml literal with each arg
* representing one line of the Yaml file
* Does not support any SD/AD features.
*
* @param yamlConfigContents the YAML configuration contents, each representing a configuration file
*/
protected void initApplicationWithYamlLines(String... yamlLines)
throws IOException {
String yamlConfig = ConfigUtil.concatWithNewlines(yamlLines);
Path tempFile = ConfigUtil.writeConfigYamlToTemp(yamlConfig, "config");
protected void initApplicationWithYamlLines(Path... yamlConfigs) {
if (yamlConfigs == null || yamlConfigs.length == 0) {
throw new IllegalArgumentException("yamlConfigContents cannot be null or empty");
}

String confdDirectory = tempFile.getParent().toString();
String yamlFileName = tempFile.getFileName().toString();
String confdDirectory = yamlConfigs[0].getParent().toString();

List<String> params = new ArrayList<String>();
List<String> params = new ArrayList<>();
params.add("--reporter");
params.add("console");

if (confdDirectory != null) {
for (Path yamlConfig : yamlConfigs) {
params.add("-c");
params.add(yamlFileName);
params.add("--conf_directory");
params.add(confdDirectory);
params.add("collect");
params.add(yamlConfig.getFileName().toString());
}
new JCommander(appConfig, params.toArray(new String[params.size()]));

params.add("--conf_directory");
params.add(confdDirectory);
params.add("collect");

new JCommander(appConfig, params.toArray(new String[0]));
this.app = new App(appConfig);
app.init(false);
}

public Path makeTempYamlConfigFile(String yamlName, List<String> lines) {
try {
return Files.write(
Files.createTempFile(temporaryFolder.getRoot().toPath(), yamlName, ".yaml"),
lines,
StandardCharsets.UTF_8
);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

/** Run a JMXFetch iteration. */
protected void run() {
Expand Down Expand Up @@ -255,7 +244,7 @@ protected List<Map<String, Object>> getServiceChecks() {
* @param additionalTags metric tags inherited from the bean properties
* @param countTags number of metric tags
* @param metricType type of the metric (gauge, histogram, ...)
* @return fail if the metric was not found
* @throws AssertionError if the metric was not found
*/
public void assertMetric(
String name,
Expand Down
Loading
Loading