From 0cc7d48981f5d5c503d503c11d42e7f3b98c2205 Mon Sep 17 00:00:00 2001 From: Gerard Downes Date: Wed, 15 Dec 2021 12:30:43 +0000 Subject: [PATCH 1/5] Replace SolrInfoBean in NRMetric with String. Since SolrInfoBean is only used to get the name of the bean which is now past directed as String. https://github.com/newrelic/newrelic-java-agent/issues/355 --- .../java/com/agent/instrumentation/solr/CacheMetric.java | 4 ++-- .../java/com/agent/instrumentation/solr/GaugeMetric.java | 4 ++-- .../com/agent/instrumentation/solr/MeteredMetric.java | 4 ++-- .../java/com/agent/instrumentation/solr/NRMetric.java | 6 +++--- .../solr/metrics/SolrMetricManager_Instrumentation.java | 6 +++--- .../java/com/agent/instrumentation/solr/CacheMetric.java | 4 ++-- .../java/com/agent/instrumentation/solr/GaugeMetric.java | 4 ++-- .../com/agent/instrumentation/solr/MeteredMetric.java | 4 ++-- .../java/com/agent/instrumentation/solr/NRMetric.java | 8 +++----- .../solr/metrics/SolrMetricManager_Instrumentation.java | 6 +++--- 10 files changed, 24 insertions(+), 26 deletions(-) diff --git a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/CacheMetric.java b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/CacheMetric.java index 7930f2d8ca..ce2efb4ea0 100644 --- a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/CacheMetric.java +++ b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/CacheMetric.java @@ -19,7 +19,7 @@ public class CacheMetric extends NRMetric { MetricsMap metric = null; String metricType = null; - public CacheMetric(String mt, String r, Metric m, SolrInfoBean b) { + public CacheMetric(String mt, String r, Metric m, String b) { super(r, b); metricType = mt; if (MetricsMap.class.isInstance(m)) { @@ -49,7 +49,7 @@ public int reportMetrics() { @Override public String getMetricBase() { - return prefix + registry + "/" + metricType + "/" + info.getName(); + return prefix + registry + "/" + metricType + "/" + name; } } diff --git a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/GaugeMetric.java b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/GaugeMetric.java index a03aa25f77..c4e84e9628 100644 --- a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/GaugeMetric.java +++ b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/GaugeMetric.java @@ -19,7 +19,7 @@ public class GaugeMetric extends NRMetric { String metricName; @SuppressWarnings("rawtypes") - public GaugeMetric(String mn, String mt, String r, Gauge m, SolrInfoBean b) { + public GaugeMetric(String mn, String mt, String r, Gauge m, String b) { super(r, b); metric = m; metricType = mt; @@ -33,7 +33,7 @@ public String getMetricName(String name) { @Override public String getMetricBase() { - return prefix + registry + "/" + metricType + "/" + info.getName(); + return prefix + registry + "/" + metricType + "/" + name; } @Override diff --git a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MeteredMetric.java b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MeteredMetric.java index 6151d0d3c3..1d84fd12b0 100644 --- a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MeteredMetric.java +++ b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MeteredMetric.java @@ -17,7 +17,7 @@ public class MeteredMetric extends NRMetric { String metricType; String metricName; - public MeteredMetric(String mn, String mt, String r, SolrInfoBean b, Metered m) { + public MeteredMetric(String mn, String mt, String r, String b, Metered m) { super(r, b); metered = m; metricType = mt; @@ -31,7 +31,7 @@ public String getMetricName(String name) { @Override public String getMetricBase() { - return prefix + registry + "/" + metricType + "/" + info.getName(); + return prefix + registry + "/" + metricType + "/" + name; } @Override diff --git a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java index 3b2f5fd38d..cee72a495d 100644 --- a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java +++ b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java @@ -13,14 +13,14 @@ public abstract class NRMetric { protected static final String prefix = "JMX/solr/"; - public NRMetric(String r, SolrInfoBean b) { + public NRMetric(String r, String b) { registry = r; - info = b; + name = b; } protected String registry; - protected SolrInfoBean info; + protected String name; public String getRegistry() { return registry; diff --git a/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java b/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java index da9549794b..a4207ef63a 100644 --- a/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java +++ b/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java @@ -37,12 +37,12 @@ public void register(SolrInfoBean info, String registry, Metric metric, boolean if (isCacheMetric) { MetricsMap mMap = (MetricsMap) metric; - CacheMetric nrMetric = new CacheMetric(desired, MetricUtil.getRegistry(registry), mMap, info); + CacheMetric nrMetric = new CacheMetric(desired, MetricUtil.getRegistry(registry), mMap, info.getName()); NewRelic.getAgent().getLogger().log(Level.FINEST, "Created CacheMetric of name {0}", metricName); MetricUtil.addMetric(nrMetric); } else if (isGaugeMetric) { Gauge gauge = (Gauge) metric; - GaugeMetric gMetric = new GaugeMetric(metricName, desired, MetricUtil.getRegistry(registry), gauge, info); + GaugeMetric gMetric = new GaugeMetric(metricName, desired, MetricUtil.getRegistry(registry), gauge, info.getName()); NewRelic.getAgent().getLogger().log(Level.FINEST, "Created GaugeMetric of name {0}", metricName); MetricUtil.addMetric(gMetric); } @@ -56,7 +56,7 @@ public Meter meter(SolrInfoBean info, String registry, String metricName, String if (MetricUtil.isDesired(metricName, metricPath)) { String mName = MetricUtil.getRemap(metricName); String desired = MetricUtil.getDesired(metricName, metricPath); - MeteredMetric meteredMetric = new MeteredMetric(mName, desired, MetricUtil.getRegistry(registry), info, meter); + MeteredMetric meteredMetric = new MeteredMetric(mName, desired, MetricUtil.getRegistry(registry), info.getName(), meter); MetricUtil.addMetric(meteredMetric); NewRelic.getAgent().getLogger().log(Level.FINEST, "Added NRMetric from ({0}, {1}, {2})", info, registry, metricName); } diff --git a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/CacheMetric.java b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/CacheMetric.java index 7930f2d8ca..ce2efb4ea0 100644 --- a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/CacheMetric.java +++ b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/CacheMetric.java @@ -19,7 +19,7 @@ public class CacheMetric extends NRMetric { MetricsMap metric = null; String metricType = null; - public CacheMetric(String mt, String r, Metric m, SolrInfoBean b) { + public CacheMetric(String mt, String r, Metric m, String b) { super(r, b); metricType = mt; if (MetricsMap.class.isInstance(m)) { @@ -49,7 +49,7 @@ public int reportMetrics() { @Override public String getMetricBase() { - return prefix + registry + "/" + metricType + "/" + info.getName(); + return prefix + registry + "/" + metricType + "/" + name; } } diff --git a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/GaugeMetric.java b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/GaugeMetric.java index a03aa25f77..c4e84e9628 100644 --- a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/GaugeMetric.java +++ b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/GaugeMetric.java @@ -19,7 +19,7 @@ public class GaugeMetric extends NRMetric { String metricName; @SuppressWarnings("rawtypes") - public GaugeMetric(String mn, String mt, String r, Gauge m, SolrInfoBean b) { + public GaugeMetric(String mn, String mt, String r, Gauge m, String b) { super(r, b); metric = m; metricType = mt; @@ -33,7 +33,7 @@ public String getMetricName(String name) { @Override public String getMetricBase() { - return prefix + registry + "/" + metricType + "/" + info.getName(); + return prefix + registry + "/" + metricType + "/" + name; } @Override diff --git a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MeteredMetric.java b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MeteredMetric.java index 6151d0d3c3..1d84fd12b0 100644 --- a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MeteredMetric.java +++ b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MeteredMetric.java @@ -17,7 +17,7 @@ public class MeteredMetric extends NRMetric { String metricType; String metricName; - public MeteredMetric(String mn, String mt, String r, SolrInfoBean b, Metered m) { + public MeteredMetric(String mn, String mt, String r, String b, Metered m) { super(r, b); metered = m; metricType = mt; @@ -31,7 +31,7 @@ public String getMetricName(String name) { @Override public String getMetricBase() { - return prefix + registry + "/" + metricType + "/" + info.getName(); + return prefix + registry + "/" + metricType + "/" + name; } @Override diff --git a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java index 3b2f5fd38d..13ee997248 100644 --- a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java +++ b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java @@ -7,20 +7,18 @@ package com.agent.instrumentation.solr; -import org.apache.solr.core.SolrInfoBean; - public abstract class NRMetric { protected static final String prefix = "JMX/solr/"; - public NRMetric(String r, SolrInfoBean b) { + public NRMetric(String r, String b) { registry = r; - info = b; + name = b; } protected String registry; - protected SolrInfoBean info; + protected String name; public String getRegistry() { return registry; diff --git a/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java b/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java index 7d299dcead..30297c5b6f 100644 --- a/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java +++ b/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java @@ -37,12 +37,12 @@ public void registerMetric(SolrInfoBean info, String registry, Metric metric, bo if (isCacheMetric) { MetricsMap mMap = (MetricsMap) metric; - CacheMetric nrMetric = new CacheMetric(desired, MetricUtil.getRegistry(registry), mMap, info); + CacheMetric nrMetric = new CacheMetric(desired, MetricUtil.getRegistry(registry), mMap, info.getName()); NewRelic.getAgent().getLogger().log(Level.FINEST, "Created CacheMetric of name {0}", metricName); MetricUtil.addMetric(nrMetric); } else if (isGaugeMetric) { Gauge gauge = (Gauge) metric; - GaugeMetric gMetric = new GaugeMetric(metricName, desired, MetricUtil.getRegistry(registry), gauge, info); + GaugeMetric gMetric = new GaugeMetric(metricName, desired, MetricUtil.getRegistry(registry), gauge, info.getName()); NewRelic.getAgent().getLogger().log(Level.FINEST, "Created GaugeMetric of name {0}", metricName); MetricUtil.addMetric(gMetric); } @@ -56,7 +56,7 @@ public Meter meter(SolrInfoBean info, String registry, String metricName, String if (MetricUtil.isDesired(metricName, metricPath)) { String mName = MetricUtil.getRemap(metricName); String desired = MetricUtil.getDesired(metricName, metricPath); - MeteredMetric meteredMetric = new MeteredMetric(mName, desired, MetricUtil.getRegistry(registry), info, meter); + MeteredMetric meteredMetric = new MeteredMetric(mName, desired, MetricUtil.getRegistry(registry), info.getName(), meter); MetricUtil.addMetric(meteredMetric); NewRelic.getAgent().getLogger().log(Level.FINEST, "Added NRMetric from ({0}, {1}, {2})", info, registry, metricName); } From e441002f6f4094a7a42176628d5a10faa409cb59 Mon Sep 17 00:00:00 2001 From: Gerard Downes Date: Fri, 17 Dec 2021 10:32:49 +0000 Subject: [PATCH 2/5] Add additional instrumentation to handle removing and moving of metrics to prevent metric build up. Added unit tests for SolrMetricManager https://github.com/newrelic/newrelic-java-agent/issues/355 --- .../instrumentation/solr/MetricUtil.java | 27 ++++ .../agent/instrumentation/solr/NRMetric.java | 6 +- .../SolrMetricManager_Instrumentation.java | 25 ++++ ...SolrMetricManagerInstrumentationTests.java | 121 ++++++++++++++++++ .../instrumentation/solr/MetricUtil.java | 27 ++++ .../agent/instrumentation/solr/NRMetric.java | 4 + .../SolrMetricManager_Instrumentation.java | 26 ++++ ...SolrMetricManagerInstrumentationTests.java | 121 ++++++++++++++++++ 8 files changed, 355 insertions(+), 2 deletions(-) create mode 100644 instrumentation/solr-jmx-7.0.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java create mode 100644 instrumentation/solr-jmx-7.4.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java diff --git a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java index b1e746c948..585479248e 100644 --- a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java +++ b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java @@ -8,6 +8,7 @@ package com.agent.instrumentation.solr; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.StringTokenizer; @@ -48,6 +49,32 @@ public static void addMetric(NRMetric metric) { metrics.put(metricBase, metric); } + public static void removeMetric(String registry, String... metricPath) { + metrics.entrySet() + .stream() + .filter(entry -> entry.getValue().registry.equals(registry) && Arrays.stream(metricPath).anyMatch(path -> path.startsWith(entry.getValue().name))) + .forEach(x -> metrics.remove(x.getKey())); + } + + public static void swapRegistries(String sourceRegistry, String targetRegistry) { + metrics.entrySet() + .stream() + .filter(entry -> entry.getValue().registry.equals(sourceRegistry)) + .forEach(x -> { + NRMetric metric = x.getValue(); + metric.setRegistry(targetRegistry); + addMetric(metric); + metrics.remove(x.getKey()); + }); + } + + public static void clearRegistry(String registry) { + metrics.entrySet() + .stream() + .filter(entry -> entry.getValue().registry.equals(registry)) + .forEach(x -> metrics.remove(x.getKey())); + } + public static String getRegistry(String r) { if (r.startsWith(REGISTRY_PREFIX)) { return r.substring(REGISTRY_PREFIX.length() + 1); diff --git a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java index cee72a495d..551b3c1fab 100644 --- a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java +++ b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java @@ -7,8 +7,6 @@ package com.agent.instrumentation.solr; -import org.apache.solr.core.SolrInfoBean; - public abstract class NRMetric { protected static final String prefix = "JMX/solr/"; @@ -26,6 +24,10 @@ public String getRegistry() { return registry; } + public void setRegistry(String registry) { + this.registry = registry; + } + public abstract String getMetricName(String name); public abstract int reportMetrics(); diff --git a/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java b/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java index a4207ef63a..9c08ade949 100644 --- a/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java +++ b/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java @@ -21,6 +21,7 @@ import com.newrelic.api.agent.weaver.Weaver; import org.apache.solr.core.SolrInfoBean; +import java.util.Set; import java.util.logging.Level; @Weave(originalName = "org.apache.solr.metrics.SolrMetricManager", type = MatchType.ExactClass) @@ -63,4 +64,28 @@ public Meter meter(SolrInfoBean info, String registry, String metricName, String return meter; } + public void removeRegistry(String registry) { + Weaver.callOriginal(); + MetricUtil.clearRegistry(registry); + } + + public void clearRegistry(String registry) { + Weaver.callOriginal(); + MetricUtil.clearRegistry(registry); + } + + public Set clearMetrics(String registry, String... metricPath) { + Set removedMetrics = Weaver.callOriginal(); + if(removedMetrics != null) { + for (String removedMetric: removedMetrics) { + MetricUtil.removeMetric(registry, removedMetric); + } + } + return removedMetrics; + } + + public void swapRegistries(String registry1, String registry2) { + Weaver.callOriginal(); + MetricUtil.swapRegistries(registry1, registry2); + } } diff --git a/instrumentation/solr-jmx-7.0.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java b/instrumentation/solr-jmx-7.0.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java new file mode 100644 index 0000000000..d27d025225 --- /dev/null +++ b/instrumentation/solr-jmx-7.0.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java @@ -0,0 +1,121 @@ +package com.agent.instrumentation.solr; + +import com.newrelic.agent.bridge.AgentBridge; +import com.newrelic.agent.bridge.NoOpPrivateApi; +import com.newrelic.agent.introspec.InstrumentationTestConfig; +import com.newrelic.agent.introspec.InstrumentationTestRunner; + +import org.apache.solr.core.SolrInfoBean; +import org.apache.solr.metrics.MetricsMap; +import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.store.blockcache.Metrics; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.stream.Collectors; + +import static org.junit.Assert.assertEquals; + + +@RunWith(InstrumentationTestRunner.class) +@InstrumentationTestConfig(includePrefixes = { "org.apache.solr.metrics" }) +public class SolrMetricManagerInstrumentationTests { + + @Before + public void before() { + MetricUtil.clearRegistry("exampleRegistry"); + MetricUtil.clearRegistry("targetRegistry"); + } + + @Test + public void register() { + //Given + AgentBridge.privateApi = new NoOpPrivateApi(); + SolrMetricManager solrMetricManager = new SolrMetricManager(); + SolrInfoBean solrInfoBean = new Metrics(); + MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100)); + + //When + solrMetricManager.register(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache"); + + //Then + int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum(); + assertEquals(1, metricsReported); + + String registryName = MetricUtil.getMetrics().values().stream().map(x -> x.registry).collect(Collectors.joining()); + assertEquals("exampleRegistry", registryName); + } + + @Test + public void removeRegistry() { + //Given + AgentBridge.privateApi = new NoOpPrivateApi(); + SolrMetricManager solrMetricManager = new SolrMetricManager(); + SolrInfoBean solrInfoBean = new Metrics(); + MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100)); + + //When + solrMetricManager.register(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache"); + solrMetricManager.removeRegistry("exampleRegistry"); + + //Then + int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum(); + assertEquals(0, metricsReported); + } + + @Test + public void clearRegistry() { + //Given + AgentBridge.privateApi = new NoOpPrivateApi(); + SolrMetricManager solrMetricManager = new SolrMetricManager(); + SolrInfoBean solrInfoBean = new Metrics(); + MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100)); + + //When + solrMetricManager.register(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache"); + solrMetricManager.clearRegistry("exampleRegistry"); + + //Then + int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum(); + assertEquals(0, metricsReported); + } + + @Test + public void clearMetrics() { + //Given + AgentBridge.privateApi = new NoOpPrivateApi(); + SolrMetricManager solrMetricManager = new SolrMetricManager(); + SolrInfoBean solrInfoBean = new Metrics(); + MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100)); + + //When + solrMetricManager.register(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache"); + solrMetricManager.clearMetrics("exampleRegistry", "hdfsBlockCache"); + + //Then + int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum(); + assertEquals(0, metricsReported); + } + + @Test + public void swapRegistries() { + //Given + AgentBridge.privateApi = new NoOpPrivateApi(); + SolrMetricManager solrMetricManager = new SolrMetricManager(); + SolrInfoBean solrInfoBean = new Metrics(); + MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100)); + + //When + solrMetricManager.register(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache"); + solrMetricManager.swapRegistries("exampleRegistry", "targetRegistry"); + + //Then + int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum(); + assertEquals(1, metricsReported); + + String registryName = MetricUtil.getMetrics().values().stream().map(x -> x.registry).collect(Collectors.joining()); + assertEquals("targetRegistry", registryName); + } +} diff --git a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java index b1e746c948..585479248e 100644 --- a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java +++ b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java @@ -8,6 +8,7 @@ package com.agent.instrumentation.solr; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.StringTokenizer; @@ -48,6 +49,32 @@ public static void addMetric(NRMetric metric) { metrics.put(metricBase, metric); } + public static void removeMetric(String registry, String... metricPath) { + metrics.entrySet() + .stream() + .filter(entry -> entry.getValue().registry.equals(registry) && Arrays.stream(metricPath).anyMatch(path -> path.startsWith(entry.getValue().name))) + .forEach(x -> metrics.remove(x.getKey())); + } + + public static void swapRegistries(String sourceRegistry, String targetRegistry) { + metrics.entrySet() + .stream() + .filter(entry -> entry.getValue().registry.equals(sourceRegistry)) + .forEach(x -> { + NRMetric metric = x.getValue(); + metric.setRegistry(targetRegistry); + addMetric(metric); + metrics.remove(x.getKey()); + }); + } + + public static void clearRegistry(String registry) { + metrics.entrySet() + .stream() + .filter(entry -> entry.getValue().registry.equals(registry)) + .forEach(x -> metrics.remove(x.getKey())); + } + public static String getRegistry(String r) { if (r.startsWith(REGISTRY_PREFIX)) { return r.substring(REGISTRY_PREFIX.length() + 1); diff --git a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java index 13ee997248..551b3c1fab 100644 --- a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java +++ b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/NRMetric.java @@ -24,6 +24,10 @@ public String getRegistry() { return registry; } + public void setRegistry(String registry) { + this.registry = registry; + } + public abstract String getMetricName(String name); public abstract int reportMetrics(); diff --git a/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java b/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java index 30297c5b6f..3b0d6e2eff 100644 --- a/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java +++ b/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java @@ -21,6 +21,7 @@ import com.newrelic.api.agent.weaver.Weaver; import org.apache.solr.core.SolrInfoBean; +import java.util.Set; import java.util.logging.Level; @Weave(originalName = "org.apache.solr.metrics.SolrMetricManager", type = MatchType.ExactClass) @@ -62,4 +63,29 @@ public Meter meter(SolrInfoBean info, String registry, String metricName, String } return meter; } + + public void removeRegistry(String registry) { + Weaver.callOriginal(); + MetricUtil.clearRegistry(registry); + } + + public void clearRegistry(String registry) { + Weaver.callOriginal(); + MetricUtil.clearRegistry(registry); + } + + public Set clearMetrics(String registry, String... metricPath) { + Set removedMetrics = Weaver.callOriginal(); + if(removedMetrics != null) { + for (String removedMetric: removedMetrics) { + MetricUtil.removeMetric(registry, removedMetric); + } + } + return removedMetrics; + } + + public void swapRegistries(String registry1, String registry2) { + Weaver.callOriginal(); + MetricUtil.swapRegistries(registry1, registry2); + } } diff --git a/instrumentation/solr-jmx-7.4.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java b/instrumentation/solr-jmx-7.4.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java new file mode 100644 index 0000000000..635ca20730 --- /dev/null +++ b/instrumentation/solr-jmx-7.4.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java @@ -0,0 +1,121 @@ +package com.agent.instrumentation.solr; + +import com.newrelic.agent.bridge.AgentBridge; +import com.newrelic.agent.bridge.NoOpPrivateApi; +import com.newrelic.agent.introspec.InstrumentationTestConfig; +import com.newrelic.agent.introspec.InstrumentationTestRunner; + +import org.apache.solr.core.SolrInfoBean; +import org.apache.solr.metrics.MetricsMap; +import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.store.blockcache.Metrics; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.stream.Collectors; + +import static org.junit.Assert.assertEquals; + + +@RunWith(InstrumentationTestRunner.class) +@InstrumentationTestConfig(includePrefixes = { "org.apache.solr.metrics" }) +public class SolrMetricManagerInstrumentationTests { + + @Before + public void before() { + MetricUtil.clearRegistry("exampleRegistry"); + MetricUtil.clearRegistry("targetRegistry"); + } + + @Test + public void register() { + //Given + AgentBridge.privateApi = new NoOpPrivateApi(); + SolrMetricManager solrMetricManager = new SolrMetricManager(); + SolrInfoBean solrInfoBean = new Metrics(); + MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100)); + + //When + solrMetricManager.registerMetric(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache"); + + //Then + int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum(); + assertEquals(1, metricsReported); + + String registryName = MetricUtil.getMetrics().values().stream().map(x -> x.registry).collect(Collectors.joining()); + assertEquals("exampleRegistry", registryName); + } + + @Test + public void removeRegistry() { + //Given + AgentBridge.privateApi = new NoOpPrivateApi(); + SolrMetricManager solrMetricManager = new SolrMetricManager(); + SolrInfoBean solrInfoBean = new Metrics(); + MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100)); + + //When + solrMetricManager.registerMetric(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache"); + solrMetricManager.removeRegistry("exampleRegistry"); + + //Then + int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum(); + assertEquals(0, metricsReported); + } + + @Test + public void clearRegistry() { + //Given + AgentBridge.privateApi = new NoOpPrivateApi(); + SolrMetricManager solrMetricManager = new SolrMetricManager(); + SolrInfoBean solrInfoBean = new Metrics(); + MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100)); + + //When + solrMetricManager.registerMetric(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache"); + solrMetricManager.clearRegistry("exampleRegistry"); + + //Then + int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum(); + assertEquals(0, metricsReported); + } + + @Test + public void clearMetrics() { + //Given + AgentBridge.privateApi = new NoOpPrivateApi(); + SolrMetricManager solrMetricManager = new SolrMetricManager(); + SolrInfoBean solrInfoBean = new Metrics(); + MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100)); + + //When + solrMetricManager.registerMetric(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache"); + solrMetricManager.clearMetrics("exampleRegistry", "hdfsBlockCache"); + + //Then + int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum(); + assertEquals(0, metricsReported); + } + + @Test + public void swapRegistries() { + //Given + AgentBridge.privateApi = new NoOpPrivateApi(); + SolrMetricManager solrMetricManager = new SolrMetricManager(); + SolrInfoBean solrInfoBean = new Metrics(); + MetricsMap metricsMap = new MetricsMap((detailed, map) -> map.put("exampleMetric", 100)); + + //When + solrMetricManager.registerMetric(solrInfoBean, "exampleRegistry", metricsMap, false,"filterCache", "hdfsBlockCache"); + solrMetricManager.swapRegistries("exampleRegistry", "targetRegistry"); + + //Then + int metricsReported = MetricUtil.getMetrics().values().stream().map(NRMetric::reportMetrics).mapToInt(Integer::intValue).sum(); + assertEquals(1, metricsReported); + + String registryName = MetricUtil.getMetrics().values().stream().map(x -> x.registry).collect(Collectors.joining()); + assertEquals("targetRegistry", registryName); + } +} From e2b71207e541d54f7740d77fa5166b2e62cd013b Mon Sep 17 00:00:00 2001 From: Gerard Downes Date: Fri, 17 Dec 2021 14:24:02 +0000 Subject: [PATCH 3/5] Additional logging to increase awareness of registry actions https://github.com/newrelic/newrelic-java-agent/issues/355 --- .../solr/metrics/SolrMetricManager_Instrumentation.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java b/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java index 3b0d6e2eff..513590c3c2 100644 --- a/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java +++ b/instrumentation/solr-jmx-7.4.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java @@ -67,11 +67,13 @@ public Meter meter(SolrInfoBean info, String registry, String metricName, String public void removeRegistry(String registry) { Weaver.callOriginal(); MetricUtil.clearRegistry(registry); + NewRelic.getAgent().getLogger().log(Level.FINEST, "Removed {0} metric registry", registry); } public void clearRegistry(String registry) { Weaver.callOriginal(); MetricUtil.clearRegistry(registry); + NewRelic.getAgent().getLogger().log(Level.FINEST, "Cleared {0} metric registry", registry); } public Set clearMetrics(String registry, String... metricPath) { @@ -80,6 +82,7 @@ public Set clearMetrics(String registry, String... metricPath) { for (String removedMetric: removedMetrics) { MetricUtil.removeMetric(registry, removedMetric); } + NewRelic.getAgent().getLogger().log(Level.FINEST, "Cleared {0} metrics from {1} metric registry", removedMetrics.size(), registry); } return removedMetrics; } @@ -87,5 +90,6 @@ public Set clearMetrics(String registry, String... metricPath) { public void swapRegistries(String registry1, String registry2) { Weaver.callOriginal(); MetricUtil.swapRegistries(registry1, registry2); + NewRelic.getAgent().getLogger().log(Level.FINEST, "Swapped {0} metric registry to {1} metric registry", registry1, registry2); } } From bb3defe35c5adbca8d1d5e76f49e77a22dba6d54 Mon Sep 17 00:00:00 2001 From: Gerard Downes Date: Fri, 17 Dec 2021 16:16:31 +0000 Subject: [PATCH 4/5] Additional logic to handle registry prefix https://github.com/newrelic/newrelic-java-agent/issues/355 --- .../java/com/agent/instrumentation/solr/MetricUtil.java | 7 ++++--- .../java/com/agent/instrumentation/solr/MetricUtil.java | 7 ++++--- .../solr/SolrMetricManagerInstrumentationTests.java | 3 +++ 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java index 585479248e..4cc99fd922 100644 --- a/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java +++ b/instrumentation/solr-jmx-7.0.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java @@ -59,12 +59,13 @@ public static void removeMetric(String registry, String... metricPath) { public static void swapRegistries(String sourceRegistry, String targetRegistry) { metrics.entrySet() .stream() - .filter(entry -> entry.getValue().registry.equals(sourceRegistry)) + .filter(entry -> entry.getValue().registry.equals(getRegistry(sourceRegistry))) .forEach(x -> { + String currentKey = x.getKey(); NRMetric metric = x.getValue(); - metric.setRegistry(targetRegistry); + metric.setRegistry(getRegistry(targetRegistry)); addMetric(metric); - metrics.remove(x.getKey()); + metrics.remove(currentKey); }); } diff --git a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java index 585479248e..4cc99fd922 100644 --- a/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java +++ b/instrumentation/solr-jmx-7.4.0/src/main/java/com/agent/instrumentation/solr/MetricUtil.java @@ -59,12 +59,13 @@ public static void removeMetric(String registry, String... metricPath) { public static void swapRegistries(String sourceRegistry, String targetRegistry) { metrics.entrySet() .stream() - .filter(entry -> entry.getValue().registry.equals(sourceRegistry)) + .filter(entry -> entry.getValue().registry.equals(getRegistry(sourceRegistry))) .forEach(x -> { + String currentKey = x.getKey(); NRMetric metric = x.getValue(); - metric.setRegistry(targetRegistry); + metric.setRegistry(getRegistry(targetRegistry)); addMetric(metric); - metrics.remove(x.getKey()); + metrics.remove(currentKey); }); } diff --git a/instrumentation/solr-jmx-7.4.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java b/instrumentation/solr-jmx-7.4.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java index 635ca20730..d488f8300c 100644 --- a/instrumentation/solr-jmx-7.4.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java +++ b/instrumentation/solr-jmx-7.4.0/src/test/java/com/agent/instrumentation/solr/SolrMetricManagerInstrumentationTests.java @@ -117,5 +117,8 @@ public void swapRegistries() { String registryName = MetricUtil.getMetrics().values().stream().map(x -> x.registry).collect(Collectors.joining()); assertEquals("targetRegistry", registryName); + + String metricBase = MetricUtil.getMetrics().values().stream().map(NRMetric::getMetricBase).collect(Collectors.joining()); + assertEquals("JMX/solr/targetRegistry/filterCache/hdfsBlockCache", metricBase); } } From c430d8544d0970fe470d97e205039e590f23955d Mon Sep 17 00:00:00 2001 From: Gerard Downes Date: Fri, 17 Dec 2021 16:35:03 +0000 Subject: [PATCH 5/5] Additional logging to add more context at finest detail for debugging https://github.com/newrelic/newrelic-java-agent/issues/355 --- .../solr/metrics/SolrMetricManager_Instrumentation.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java b/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java index 9c08ade949..0c5336c4a3 100644 --- a/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java +++ b/instrumentation/solr-jmx-7.0.0/src/main/java/org/apache/solr/metrics/SolrMetricManager_Instrumentation.java @@ -67,11 +67,13 @@ public Meter meter(SolrInfoBean info, String registry, String metricName, String public void removeRegistry(String registry) { Weaver.callOriginal(); MetricUtil.clearRegistry(registry); + NewRelic.getAgent().getLogger().log(Level.FINEST, "Removed {0} metric registry", registry); } public void clearRegistry(String registry) { Weaver.callOriginal(); MetricUtil.clearRegistry(registry); + NewRelic.getAgent().getLogger().log(Level.FINEST, "Cleared {0} metric registry", registry); } public Set clearMetrics(String registry, String... metricPath) { @@ -80,6 +82,7 @@ public Set clearMetrics(String registry, String... metricPath) { for (String removedMetric: removedMetrics) { MetricUtil.removeMetric(registry, removedMetric); } + NewRelic.getAgent().getLogger().log(Level.FINEST, "Cleared {0} metrics from {1} metric registry", removedMetrics.size(), registry); } return removedMetrics; } @@ -87,5 +90,6 @@ public Set clearMetrics(String registry, String... metricPath) { public void swapRegistries(String registry1, String registry2) { Weaver.callOriginal(); MetricUtil.swapRegistries(registry1, registry2); + NewRelic.getAgent().getLogger().log(Level.FINEST, "Swapped {0} metric registry to {1} metric registry", registry1, registry2); } }