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.

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

Carlosroman/amlii 1815 embedded mode fix investigation - DO NOT MERGE #523

Closed
wants to merge 6 commits into from
Closed

Carlosroman/amlii 1815 embedded mode fix investigation - DO NOT MERGE #523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3f46148
Fixing logger setup in TestCommon
carlosroman May 31, 2024
7ce249c
Removing src/test/java/org/datadog/jmxfetch/TestInstance.java
carlosroman May 31, 2024
54156ad
Removing src/test/java/org/datadog/jmxfetch/TestServiceChecks.java
carlosroman May 31, 2024
f25e050
Removing src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java
carlosroman May 31, 2024
fc57775
Removing src/test/java/org/datadog/jmxfetch/TestGCMetrics.java
carlosroman May 31, 2024
1d9ce49
Will this break?
carlosroman May 31, 2024
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
19 changes: 14 additions & 5 deletions 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 @@ -133,7 +133,10 @@ public App(final AppConfig appConfig) {
}
this.configs = getConfigs(this.appConfig);

this.initTelemetryBean();
if (this.appConfig.getJmxfetchTelemetry()) {
log.info("Enabling JMX Fetch Telemetry");
this.initTelemetryBean();
}
}

private ObjectName getAppTelemetryBeanName() {
Expand Down Expand Up @@ -168,14 +171,19 @@ private void initTelemetryBean() {
| MBeanRegistrationException
| NotCompliantMBeanException e) {
log.warn("Could not register bean named '{}' for instance: ",
appTelemetryBeanName.toString(), e);
appTelemetryBeanName, e);
}

this.appTelemetry = bean;
return;
}

private void teardownTelemetry() {
if (!this.appConfig.getJmxfetchTelemetry()) {
log.debug("Skipping teardown telemetry as not enabled");
return;
}

MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
ObjectName appTelemetryBeanName = getAppTelemetryBeanName();
if (appTelemetryBeanName == null) {
Expand All @@ -187,7 +195,7 @@ private void teardownTelemetry() {
log.debug("Succesfully unregistered app telemetry bean");
} catch (MBeanRegistrationException | InstanceNotFoundException e) {
log.warn("Could not unregister bean named '{}' for instance: ",
appTelemetryBeanName.toString(), e);
appTelemetryBeanName, e);
}
}

Expand Down Expand Up @@ -540,7 +548,7 @@ public void doIteration() {
for (Instance instance : this.instances) {
getMetricsTasks.add(new MetricCollectionTask(instance));
}
if (this.appTelemetry != null) {
if (this.appTelemetry != null && this.appConfig.getJmxfetchTelemetry()) {
this.appTelemetry.setRunningInstanceCount(this.instances.size());
}

Expand Down Expand Up @@ -906,6 +914,7 @@ private Instance instantiate(
public void init(final boolean forceNewConnection) {
log.info("Cleaning up instances...");
this.clearInstances(this.instances);
this.instances.clear();
this.clearInstances(this.brokenInstanceMap.values());
this.brokenInstanceMap.clear();

Expand Down Expand Up @@ -1205,7 +1214,7 @@ private <T> void processFixedStatus(
this.brokenInstanceMap.remove(instance.toString());
this.instances.add(instance);

if (this.appTelemetry != null) {
if (this.appTelemetry != null && this.appConfig.getJmxfetchTelemetry()) {
this.appTelemetry.setBrokenInstanceCount(this.brokenInstanceMap.size());
this.appTelemetry.setRunningInstanceCount(this.instances.size());
}
Expand Down
11 changes: 8 additions & 3 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 @@ -240,7 +240,9 @@ public Instance(
log.info("collect_default_jvm_metrics is false - not collecting default JVM metrics");
}

instanceTelemetryBean = createInstanceTelemetryBean();
if (this.appConfig.getJmxfetchTelemetry()) {
this.instanceTelemetryBean = createInstanceTelemetryBean();
}
}

private ObjectName getObjName(String domain,String instance)
Expand Down Expand Up @@ -837,11 +839,14 @@ public boolean isLimitReached() {
}

private void cleanupTelemetryBean() {
if (this.instanceTelemetryBean == null) {
return;
}
try {
mbs.unregisterMBean(instanceTelemetryBeanName);
log.debug("Successfully unregistered bean for instance: " + this.getCheckName());
log.debug("Successfully unregistered bean for instance: {}", this.getCheckName());
} catch (MBeanRegistrationException | InstanceNotFoundException e) {
log.debug("Unable to unregister bean for instance: " + this.getCheckName());
log.debug("Unable to unregister bean for instance: {}", this.getCheckName());
}
}

Expand Down
24 changes: 17 additions & 7 deletions src/test/java/org/datadog/jmxfetch/TestApp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.when;

Expand All @@ -22,12 +23,19 @@ public class TestApp extends TestCommon {
/** Tag metrics with MBean parameters based on user supplied regex */
@Test
public void testBeanRegexTags() throws Exception {
// When we enable JMXFetch telemetry
when(appConfig.getJmxfetchTelemetry()).thenReturn(true);

// We expose a few metrics through JMX
registerMBean(
new SimpleTestJavaApp(),
"org.datadog.jmxfetch.test:type=SimpleTestJavaApp,scope=Co|olScope,host=localhost,component=");
initApplication("jmx_bean_regex_tags.yaml");

AppTelemetry tlm = app.getAppTelemetryBean();
assertNotNull("AppTelemetryBean should not be null", tlm);
assertEquals("Before run", 0, tlm.getRunningInstanceCount());

// Run the collection
run();

Expand All @@ -45,14 +53,15 @@ public void testBeanRegexTags() throws Exception {
"nonRegexTag:value");

assertMetric("this.is.100", tags, 10);

AppTelemetry tlm = app.getAppTelemetryBean();
assertEquals(1, tlm.getRunningInstanceCount());
assertEquals("Expecting one instance running", 1, tlm.getRunningInstanceCount());
}

/** Tag metrics with MBeans parameters. */
@Test
public void testBeanTags() throws Exception {
// When we enable JMXFetch telemetry
when(appConfig.getJmxfetchTelemetry()).thenReturn(true);

// We expose a few metrics through JMX
registerMBean(
new SimpleTestJavaApp(),
Expand All @@ -61,10 +70,11 @@ public void testBeanTags() throws Exception {

// Collecting metrics
run();
List<Map<String, Object>> metrics = getMetrics();

// 14 = 13 metrics from java.lang + 1 metric explicitly defined in the yaml config file
assertEquals(14, metrics.size());
assertEquals(
"Expecting 13 metrics from java.lang + 1 metric explicitly defined in the yaml config file",
14, getMetrics().size());

List<String> tags =
Arrays.asList(
Expand All @@ -79,6 +89,7 @@ public void testBeanTags() throws Exception {
assertMetric("this.is.100", tags, 7);

AppTelemetry tlm = app.getAppTelemetryBean();
assertNotNull("AppTelemetryBean should not be null", tlm);
assertEquals(1, tlm.getRunningInstanceCount());
}

Expand Down Expand Up @@ -125,10 +136,9 @@ public void testBeanTagsDontNormalizeParams() throws Exception {

// Collecting metrics
run();
List<Map<String, Object>> metrics = getMetrics();

// 14 = 13 metrics from java.lang + 1 metric explicitly defined in the yaml config file
assertEquals(14, metrics.size());
assertEquals(14, getMetrics().size());

List<String> tags =
Arrays.asList(
Expand Down
17 changes: 10 additions & 7 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 @@ -62,7 +62,7 @@ public class TestCommon {
AppConfig appConfig = spy(AppConfig.builder().build());
App app;
MBeanServer mbs;
List<ObjectName> objectNames = new ArrayList<ObjectName>();
List<ObjectName> objectNames = new ArrayList<>();
List<Map<String, Object>> metrics;
List<Map<String, Object>> serviceChecks;

Expand All @@ -73,7 +73,7 @@ public static void init() throws Exception {
if (level == null) {
level = "ALL";
}
CustomLogger.setup(LogLevel.ALL, "/tmp/jmxfetch_test.log", false);
CustomLogger.setup(LogLevel.fromString(level), "/tmp/jmxfetch_test.log", false);
}

/**
Expand All @@ -87,10 +87,12 @@ public static void init() throws Exception {
protected void registerMBean(Object application, String objectStringName)
throws InstanceAlreadyExistsException, MBeanRegistrationException,
NotCompliantMBeanException, MalformedObjectNameException {
mbs = (mbs == null) ? ManagementFactory.getPlatformMBeanServer() : mbs;
if(this.mbs == null) {
this.mbs = ManagementFactory.getPlatformMBeanServer();
}
ObjectName objectName = new ObjectName(objectStringName);
this.mbs.registerMBean(application, objectName);
objectNames.add(objectName);
mbs.registerMBean(application, objectName);
}

/**
Expand All @@ -100,10 +102,11 @@ protected void registerMBean(Object application, String objectStringName)
* @throws MBeanRegistrationException
*/
public void unregisterMBeans() throws MBeanRegistrationException, InstanceNotFoundException {
if (mbs != null) {
for (ObjectName objectName : objectNames) {
mbs.unregisterMBean(objectName);
if (this.mbs != null) {
for (ObjectName objectName : this.objectNames) {
this.mbs.unregisterMBean(objectName);
}
this.objectNames.clear();
}
}

Expand Down
Loading
Loading