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

Don't emit Current Thread Count and Loaded Class Count to OTLP #3679

Merged
merged 13 commits into from
Jun 5, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -558,33 +558,6 @@ private static void loadJmxMetricsEnvVar(
}
}

private static void addDefaultJmxMetricsIfNotPresent(Configuration config) {
if (!jmxMetricExists(config.jmxMetrics, "java.lang:type=Threading", "ThreadCount")) {
JmxMetric threadCountJmxMetric = new JmxMetric();
threadCountJmxMetric.name = "Current Thread Count";
threadCountJmxMetric.objectName = "java.lang:type=Threading";
threadCountJmxMetric.attribute = "ThreadCount";
config.jmxMetrics.add(threadCountJmxMetric);
}
if (!jmxMetricExists(config.jmxMetrics, "java.lang:type=ClassLoading", "LoadedClassCount")) {
JmxMetric classCountJmxMetric = new JmxMetric();
classCountJmxMetric.name = "Loaded Class Count";
classCountJmxMetric.objectName = "java.lang:type=ClassLoading";
classCountJmxMetric.attribute = "LoadedClassCount";
config.jmxMetrics.add(classCountJmxMetric);
}
}

private static boolean jmxMetricExists(
List<JmxMetric> jmxMetrics, String objectName, String attribute) {
for (JmxMetric metric : jmxMetrics) {
if (metric.objectName.equals(objectName) && metric.attribute.equals(attribute)) {
return true;
}
}
return false;
}

private static void overlayInstrumentationEnabledEnvVars(
Configuration config, Function<String, String> envVarsFunction) {
config.instrumentation.azureSdk.enabled =
Expand Down Expand Up @@ -844,7 +817,6 @@ static void overlayFromEnv(
loadLogCaptureEnvVar(config, envVarsFunction);
loadJmxMetricsEnvVar(config, envVarsFunction);

addDefaultJmxMetricsIfNotPresent(config);
overlayProfilerEnvVars(config, envVarsFunction);
overlayAadEnvVars(config, envVarsFunction);
overlayInstrumentationEnabledEnvVars(config, envVarsFunction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.microsoft.applicationinsights.agent.internal.perfcounter.GcPerformanceCounter;
import com.microsoft.applicationinsights.agent.internal.perfcounter.JmxAttributeData;
import com.microsoft.applicationinsights.agent.internal.perfcounter.JmxDataFetcher;
import com.microsoft.applicationinsights.agent.internal.perfcounter.JmxMetricPerformanceCounter;
import com.microsoft.applicationinsights.agent.internal.perfcounter.JvmHeapMemoryUsedPerformanceCounter;
import com.microsoft.applicationinsights.agent.internal.perfcounter.OshiPerformanceCounter;
import com.microsoft.applicationinsights.agent.internal.perfcounter.PerformanceCounterContainer;
Expand All @@ -25,6 +26,7 @@
import java.lang.management.ManagementFactory;
import java.lang.management.ThreadMXBean;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
Expand All @@ -48,6 +50,21 @@ public static void initialize(Configuration configuration) {
PerformanceCounterContainer.INSTANCE.setLogAvailableJmxMetrics();
}

// We don't want these two to be flowing to the OTLP endpoint
// because in the long term we will probably be deprecating these
// in favor of the otel instrumentation runtime metrics that relay
// the same information.
registerCounterInContainer(
"java.lang:type=Threading",
"Current Thread Count",
"ThreadCount",
configuration.jmxMetrics);
registerCounterInContainer(
"java.lang:type=ClassLoading",
"Loaded Class Count",
"LoadedClassCount",
configuration.jmxMetrics);

loadCustomJmxPerfCounters(configuration.jmxMetrics);

PerformanceCounterContainer.INSTANCE.register(
Expand All @@ -74,6 +91,29 @@ private static boolean isAgentRunningInSandboxEnvWindows() {
return qualifiedSdkVersion.startsWith("awr") || qualifiedSdkVersion.startsWith("fwr");
}

private static void registerCounterInContainer(
String objectName,
String metricName,
String attribute,
List<Configuration.JmxMetric> jmxMetricsList) {
if (!isMetricInConfig(objectName, attribute, jmxMetricsList)) {
JmxAttributeData attributeData = new JmxAttributeData(metricName, attribute);
JmxMetricPerformanceCounter jmxPerfCounter =
new JmxMetricPerformanceCounter(objectName, Arrays.asList(attributeData));
PerformanceCounterContainer.INSTANCE.register(jmxPerfCounter);
}
}

private static boolean isMetricInConfig(
String objectName, String attribute, List<Configuration.JmxMetric> jmxMetricsList) {
for (Configuration.JmxMetric metric : jmxMetricsList) {
if (metric.objectName.equals(objectName) && metric.attribute.equals(attribute)) {
return true;
}
}
return false;
}

/**
* The method will load the Jmx performance counters requested by the user to the system: 1. Build
* a map where the key is the Jmx object name and the value is a list of requested attributes. 2.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,11 @@ void shouldParseFromEnvVar() throws IOException {

List<JmxMetric> jmxMetrics = parseJmxMetricsJson(jmxMetricsJson);
assertThat(jmxMetrics.size()).isEqualTo(2);
assertThat(configuration.jmxMetrics.size()).isEqualTo(3);
assertThat(configuration.jmxMetrics.size()).isEqualTo(2);
assertThat(configuration.jmxMetrics.get(0).name)
.isEqualTo(jmxMetrics.get(0).name); // class count is overridden by the env var
assertThat(configuration.jmxMetrics.get(1).name)
.isEqualTo(jmxMetrics.get(1).name); // code cache is overridden by the env var
assertThat("Current Thread Count").isEqualTo(configuration.jmxMetrics.get(2).name);
}

@Test
Expand Down Expand Up @@ -513,12 +512,11 @@ void shouldOverrideJmxMetrics() throws IOException {

List<JmxMetric> jmxMetrics = parseJmxMetricsJson(jmxMetricsJson);
assertThat(jmxMetrics.size()).isEqualTo(2);
assertThat(configuration.jmxMetrics.size()).isEqualTo(3);
assertThat(configuration.jmxMetrics.size()).isEqualTo(2);
assertThat(configuration.jmxMetrics.get(0).name)
.isEqualTo(jmxMetrics.get(0).name); // class count is overridden by the env var
assertThat(configuration.jmxMetrics.get(1).name)
.isEqualTo(jmxMetrics.get(1).name); // code cache is overridden by the env var
assertThat("Current Thread Count").isEqualTo(configuration.jmxMetrics.get(2).name);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ abstract class DetectUnexpectedOtelMetricsTest {
EXPECTED_METRIC_NAMES.add("% Of Max Heap Memory Used");
EXPECTED_METRIC_NAMES.add("GC Total Count");
EXPECTED_METRIC_NAMES.add("GC Total Time");

EXPECTED_METRIC_NAMES.add("http.server.request.duration");
EXPECTED_METRIC_NAMES.add("http.client.request.duration");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,9 @@ abstract class JmxMetricTest {
* <p>- Higher than Java 8: G1 Young Generation,type=GarbageCollector, G1 Old
* Generation,type=GarbageCollector. (the corrresponding metric names are GCYoung and GCOld)
*
* <p>- Loaded_Class_Count: This covers the case of collecting a default jmx metric that the
* customer did not specify in applicationInsights.json. Also note that there are underscores
* instead of spaces, as we are emitting the metric via OpenTelemetry now. We intend to implement
* a change in azure-sdk-for-java repo to have the metrics still emit with spaces to Breeze, but
* the OTEL collector will still get the metric names with _. When that change gets merged &
* incorporated, we will need to change this/DetectUnexpectedOtelMetrics test so that the Breeze
* verification expects our default jmx metrics with spaces.
* <p>- Loaded Class Count: This covers the case of collecting a default jmx metric that the
Copy link
Contributor

Choose a reason for hiding this comment

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

can we mention this metric goes to breeze only now?

* customer did not specify in applicationInsights.json. TODO deprecate this in favor of analogous
* metric that is emitted via OTel instrumentation runtime metrics.
*
* <p>- BooleanJmxMetric: This covers the case of a jmx metric attribute with a boolean value.
*
Expand All @@ -81,7 +77,6 @@ abstract class JmxMetricTest {
"NameWithDot",
"DefaultJmxMetricNameOverride",
"WildcardJmxMetric",
"Loaded_Class_Count",
"BooleanJmxMetric",
"DotInAttributeNameAsPathSeparator"));

Expand All @@ -91,8 +86,8 @@ abstract class JmxMetricTest {
@Test
@TargetUri("/test")
void doMostBasicTest() throws Exception {
verifyJmxMetricsSentToBreeze();
verifyJmxMetricsSentToOtlpEndpoint();
verifyJmxMetricsSentToBreeze();
}

@SuppressWarnings({"PreferJavaTimeOverload"})
Expand Down Expand Up @@ -126,12 +121,9 @@ private void verifyJmxMetricsSentToOtlpEndpoint() {
}
}

for (Map.Entry<String, Integer> entry : occurrences.entrySet()) {
System.out.println(entry.toString());
}
// confirm that those metrics received once or twice
// (the collector seems to run for 5-10 sec)
assertThat(occurrences.keySet()).hasSize(6);
assertThat(occurrences.keySet()).hasSize(jmxMetricsAllJavaVersionsOtlp.size());
for (int value : occurrences.values()) {
assertThat(value).isBetween(1, 8);
}
Expand All @@ -140,7 +132,7 @@ private void verifyJmxMetricsSentToOtlpEndpoint() {

private void verifyJmxMetricsSentToBreeze() throws Exception {
List<Envelope> metricItems =
testing.mockedIngestion.waitForItems(JmxMetricTest::isJmxMetric, 1, 10, TimeUnit.SECONDS);
testing.mockedIngestion.waitForItems(JmxMetricTest::isJmxMetric, 7, 10, TimeUnit.SECONDS);
harsimar marked this conversation as resolved.
Show resolved Hide resolved
harsimar marked this conversation as resolved.
Show resolved Hide resolved

assertThat(metricItems).hasSizeBetween(7, 16);

Expand Down
Loading