Skip to content

Commit

Permalink
Disable jvm.gc metrics (elastic#437)
Browse files Browse the repository at this point in the history
The APM 6.6.0 UI has problems handling metrics with tags.
So for the moment tagged metrics will be disabled.
Once you are on Kibana 6.6.1, you can safely enable the metrics again.
  • Loading branch information
felixbarny authored Jan 22, 2019
1 parent 3675422 commit 1e13465
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 3 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
Example: `com.example.*` traces all classes and methods within the `com.example` package and sub-packages.
* Added support for JSF. Tested on WildFly, WebSphere Liberty and Payara with embedded JSF implementation and on Tomcat and Jetty with
MyFaces 2.2 and 2.3
* Introduces a new configuration option `disable_metrics` which disables the collection of metrics via a wildcard expression.
The default is set to `jvm.gc.*` to circumvent a bug in the APM UI in Kibana 6.6.0.
You can remove the default value once you are on Kibana 6.6.1 or higher.

## Bug Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,11 @@ protected Deque<TraceContextHolder<?>> initialValue() {
};
private final CoreConfiguration coreConfiguration;
private final List<ActivationListener> activationListeners;
private final MetricRegistry metricRegistry = new MetricRegistry();
private final MetricRegistry metricRegistry;
private Sampler sampler;

ElasticApmTracer(ConfigurationRegistry configurationRegistry, Reporter reporter, Iterable<LifecycleListener> lifecycleListeners, List<ActivationListener> activationListeners) {
this.metricRegistry = new MetricRegistry(configurationRegistry.getConfig(ReporterConfiguration.class));
this.configurationRegistry = configurationRegistry;
this.reporter = reporter;
this.stacktraceConfiguration = configurationRegistry.getConfig(StacktraceConfiguration.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
*/
package co.elastic.apm.agent.metrics;

import co.elastic.apm.agent.matcher.WildcardMatcher;
import co.elastic.apm.agent.report.ReporterConfiguration;

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
Expand All @@ -36,6 +39,11 @@ public class MetricRegistry {
* Groups {@link MetricSet}s by their unique tags.
*/
private final ConcurrentMap<Map<String, String>, MetricSet> metricSets = new ConcurrentHashMap<>();
private final ReporterConfiguration config;

public MetricRegistry(ReporterConfiguration config) {
this.config = config;
}

/**
* Same as {@link #add(String, Map, DoubleSupplier)} but only adds the metric
Expand All @@ -50,6 +58,9 @@ public class MetricRegistry {
* @see #add(String, Map, DoubleSupplier)
*/
public void addUnlessNan(String name, Map<String, String> tags, DoubleSupplier metric) {
if (isDisabled(name)) {
return;
}
if (!Double.isNaN(metric.get())) {
add(name, tags, metric);
}
Expand All @@ -68,6 +79,9 @@ public void addUnlessNan(String name, Map<String, String> tags, DoubleSupplier m
* @see #add(String, Map, DoubleSupplier)
*/
public void addUnlessNegative(String name, Map<String, String> tags, DoubleSupplier metric) {
if (isDisabled(name)) {
return;
}
if (metric.get() >= 0) {
add(name, tags, metric);
}
Expand All @@ -84,6 +98,9 @@ public void addUnlessNegative(String name, Map<String, String> tags, DoubleSuppl
* ({@link co.elastic.apm.agent.report.ReporterConfiguration#metricsInterval metrics_interval)})
*/
public void add(String name, Map<String, String> tags, DoubleSupplier metric) {
if (isDisabled(name)) {
return;
}
MetricSet metricSet = metricSets.get(tags);
if (metricSet == null) {
metricSets.putIfAbsent(tags, new MetricSet(tags));
Expand All @@ -92,6 +109,10 @@ public void add(String name, Map<String, String> tags, DoubleSupplier metric) {
metricSet.add(name, metric);
}

private boolean isDisabled(String name) {
return WildcardMatcher.anyMatch(config.getDisableMetrics(), name) != null;
}

public double get(String name, Map<String, String> tags) {
final MetricSet metricSet = metricSets.get(tags);
if (metricSet != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@
import co.elastic.apm.agent.configuration.converter.ByteValueConverter;
import co.elastic.apm.agent.configuration.converter.TimeDuration;
import co.elastic.apm.agent.configuration.converter.TimeDurationValueConverter;
import co.elastic.apm.agent.matcher.WildcardMatcher;
import co.elastic.apm.agent.matcher.WildcardMatcherValueConverter;
import org.stagemonitor.configuration.ConfigurationOption;
import org.stagemonitor.configuration.ConfigurationOptionProvider;
import org.stagemonitor.configuration.converter.ListValueConverter;
import org.stagemonitor.configuration.converter.UrlValueConverter;

import javax.annotation.Nullable;
Expand Down Expand Up @@ -144,6 +147,17 @@ public class ReporterConfiguration extends ConfigurationOptionProvider {
.addValidator(isNotInRange(TimeDuration.of("1ms"), TimeDuration.of("999ms")))
.buildWithDefault(TimeDuration.of("30s"));

private final ConfigurationOption<List<WildcardMatcher>> disableMetrics = ConfigurationOption
.builder(new ListValueConverter<>(new WildcardMatcherValueConverter()), List.class)
.key("disable_metrics")
.configurationCategory(REPORTER_CATEGORY)
.description("Disables the collection of certain metrics.\n" +
"If the name of a metric matches any of the wildcard expressions, it will not be collected.\n" +
"\n" +
WildcardMatcher.DOCUMENTATION)
.dynamic(false)
.buildWithDefault(Collections.singletonList(WildcardMatcher.valueOf("jvm.gc.*")));

@Nullable
public String getSecretToken() {
return secretToken.get();
Expand Down Expand Up @@ -188,4 +202,8 @@ public long getApiRequestSize() {
public long getMetricsIntervalMs() {
return metricsInterval.get().getMillis();
}

public List<WildcardMatcher> getDisableMetrics() {
return disableMetrics.get();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*-
* #%L
* Elastic APM Java agent
* %%
* Copyright (C) 2018 - 2019 Elastic and contributors
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/
package co.elastic.apm.agent.metrics;

import co.elastic.apm.agent.matcher.WildcardMatcher;
import co.elastic.apm.agent.report.ReporterConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.util.List;

import static java.util.Collections.emptyMap;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class MetricRegistryTest {

private MetricRegistry metricRegistry;
private ReporterConfiguration config;

@BeforeEach
void setUp() {
config = mock(ReporterConfiguration.class);
metricRegistry = new MetricRegistry(config);
}

@Test
void testDisabledMetrics() {
when(config.getDisableMetrics()).thenReturn(List.of(WildcardMatcher.valueOf("jvm.gc.*")));
final DoubleSupplier problematicMetric = () -> {
throw new RuntimeException("Huston, we have a problem");
};
metricRegistry.addUnlessNegative("jvm.gc.count", emptyMap(), problematicMetric);
metricRegistry.addUnlessNan("jvm.gc.count", emptyMap(), problematicMetric);
metricRegistry.add("jvm.gc.count", emptyMap(), problematicMetric);
assertThat(metricRegistry.getMetricSets()).isEmpty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,21 @@
package co.elastic.apm.agent.metrics.builtin;

import co.elastic.apm.agent.metrics.MetricRegistry;
import co.elastic.apm.agent.report.ReporterConfiguration;
import org.junit.jupiter.api.Test;

import java.util.Collections;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

class JvmMemoryMetricsTest {

private final JvmMemoryMetrics jvmMemoryMetrics = new JvmMemoryMetrics();

@Test
void testMetrics() {
final MetricRegistry registry = new MetricRegistry();
final MetricRegistry registry = new MetricRegistry(mock(ReporterConfiguration.class));
jvmMemoryMetrics.bindTo(registry);
System.out.println(registry.toString());
assertThat(registry.get("jvm.memory.heap.used", Collections.emptyMap())).isNotZero();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@
package co.elastic.apm.agent.metrics.builtin;

import co.elastic.apm.agent.metrics.MetricRegistry;
import co.elastic.apm.agent.report.ReporterConfiguration;
import org.junit.jupiter.api.Test;

import java.util.Collections;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

class SystemMetricsTest {

private MetricRegistry metricRegistry = new MetricRegistry();
private MetricRegistry metricRegistry = new MetricRegistry(mock(ReporterConfiguration.class));
private SystemMetrics systemMetrics = new SystemMetrics();

@Test
Expand Down
40 changes: 40 additions & 0 deletions docs/configuration.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,32 @@ The default unit for this option is `s`
| `elastic.apm.metrics_interval` | `metrics_interval` | `ELASTIC_APM_METRICS_INTERVAL`
|============

[float]
[[config-disable-metrics]]
==== `disable_metrics`

Disables the collection of certain metrics.
If the name of a metric matches any of the wildcard expressions, it will not be collected.

This option supports the wildcard `*`, which matches zero or more characters.
Examples: `/foo/*/bar/*/baz*`, `*foo*`.
Matching is case insensitive by default.
Prepending an element with `(?-i)` makes the matching case sensitive.


[options="header"]
|============
| Default | Type | Dynamic
| `jvm.gc.*` | List | false
|============


[options="header"]
|============
| Java System Properties | Property file | Environment
| `elastic.apm.disable_metrics` | `disable_metrics` | `ELASTIC_APM_DISABLE_METRICS`
|============

[[config-stacktrace]]
=== Stacktrace configuration options
[float]
Expand Down Expand Up @@ -1356,6 +1382,20 @@ The default unit for this option is `ms`
#
# metrics_interval=30s
# Disables the collection of certain metrics.
# If the name of a metric matches any of the wildcard expressions, it will not be collected.
#
# This option supports the wildcard `*`, which matches zero or more characters.
# Examples: `/foo/*/bar/*/baz*`, `*foo*`.
# Matching is case insensitive by default.
# Prepending an element with `(?-i)` makes the matching case sensitive.
#
# This setting can not be changed at runtime. Changes require a restart of the application.
# Type: comma separated list
# Default value: jvm.gc.*
#
# disable_metrics=jvm.gc.*
############################################
# Stacktrace #
############################################
Expand Down

0 comments on commit 1e13465

Please sign in to comment.