This is an automated email from the ASF dual-hosted git repository. radu pushed a commit to branch issue/SLING-11429 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git
commit 4c3ae65ab8ec902d479e079afb6bdf278624e989 Author: Radu Cotescu <[email protected]> AuthorDate: Tue Aug 30 16:21:59 2022 +0200 SLING-11429 - OSGi configs on same resource type cause IAE * use the GraphQLServlet service.pid in order to have unique instances for all the required metrics --- pom.xml | 13 +++++-------- .../sling/graphql/core/servlet/GraphQLServlet.java | 18 +++++++++++------- .../sling/graphql/core/servlet/GraphQLServletTest.java | 4 ++-- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/pom.xml b/pom.xml index 567ebe4..b0a55c1 100644 --- a/pom.xml +++ b/pom.xml @@ -52,14 +52,6 @@ <build> <plugins> - <plugin> - <groupId>biz.aQute.bnd</groupId> - <artifactId>bnd-maven-plugin</artifactId> - </plugin> - <plugin> - <groupId>biz.aQute.bnd</groupId> - <artifactId>bnd-baseline-maven-plugin</artifactId> - </plugin> <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-failsafe-plugin</artifactId> @@ -94,6 +86,11 @@ <artifactId>osgi.core</artifactId> <scope>provided</scope> </dependency> + <dependency> + <groupId>org.osgi</groupId> + <artifactId>org.osgi.service.component</artifactId> + <scope>provided</scope> + </dependency> <dependency> <groupId>org.osgi</groupId> <artifactId>org.osgi.service.component.annotations</artifactId> diff --git a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java index 9bba5a8..3e1427e 100644 --- a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java +++ b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java @@ -40,10 +40,13 @@ import org.apache.sling.api.servlets.SlingAllMethodsServlet; import org.apache.sling.commons.metrics.Counter; import org.apache.sling.commons.metrics.MetricsService; import org.apache.sling.commons.metrics.Timer; +import org.apache.sling.commons.osgi.PropertiesUtil; import org.apache.sling.graphql.api.cache.GraphQLCacheProvider; import org.apache.sling.graphql.api.engine.QueryExecutor; import org.apache.sling.graphql.api.engine.ValidationResult; import org.jetbrains.annotations.NotNull; +import org.osgi.framework.Constants; +import org.osgi.service.component.ComponentContext; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.ConfigurationPolicy; @@ -146,11 +149,12 @@ public class GraphQLServlet extends SlingAllMethodsServlet { private Counter requestsServed; private Timer requestTimer; - private static final String METRIC_NS = GraphQLServlet.class.getName(); private String gaugeCacheHitRate; @Activate - private void activate(Config config) { + private void activate(Config config, ComponentContext componentContext) { + String servicePid = PropertiesUtil.toString(componentContext.getProperties().get(Constants.SERVICE_PID), + GraphQLServlet.class.getName()); String[] extensions = config.sling_servlet_extensions(); StringBuilder extensionsPattern = new StringBuilder(); for (String extension : extensions) { @@ -192,16 +196,16 @@ public class GraphQLServlet extends SlingAllMethodsServlet { sb.append(".e:").append(String.join("_", extensions)); } String servletRegistrationProperties = sb.toString(); - cacheHits = metricsService.counter(METRIC_NS + "." + servletRegistrationProperties + ".cache_hits"); - cacheMisses = metricsService.counter(METRIC_NS + "." + servletRegistrationProperties + ".cache_misses"); - requestsServed = metricsService.counter(METRIC_NS + "." + servletRegistrationProperties + ".requests_total"); - gaugeCacheHitRate = METRIC_NS + "." + servletRegistrationProperties + ".cache_hit_rate"; + cacheHits = metricsService.counter(servicePid + "." + servletRegistrationProperties + ".cache_hits"); + cacheMisses = metricsService.counter(servicePid + "." + servletRegistrationProperties + ".cache_misses"); + requestsServed = metricsService.counter(servicePid + "." + servletRegistrationProperties + ".requests_total"); + gaugeCacheHitRate = servicePid + "." + servletRegistrationProperties + ".cache_hit_rate"; metricRegistry.register(gaugeCacheHitRate, (Gauge<Float>) () -> { float hitCount = cacheHits.getCount(); float missCount = cacheMisses.getCount(); return hitCount > 0 || missCount > 0 ? hitCount / (hitCount + missCount) : 0.0f; }); - requestTimer = metricsService.timer(METRIC_NS + "." + servletRegistrationProperties + ".requests_timer"); + requestTimer = metricsService.timer(servicePid + "." + servletRegistrationProperties + ".requests_timer"); } @Deactivate diff --git a/src/test/java/org/apache/sling/graphql/core/servlet/GraphQLServletTest.java b/src/test/java/org/apache/sling/graphql/core/servlet/GraphQLServletTest.java index 39f1f8d..6872daf 100644 --- a/src/test/java/org/apache/sling/graphql/core/servlet/GraphQLServletTest.java +++ b/src/test/java/org/apache/sling/graphql/core/servlet/GraphQLServletTest.java @@ -148,7 +148,7 @@ public class GraphQLServletTest { context.registerInjectActivateService(new GraphQLServlet(), ServletResolverConstants.SLING_SERVLET_RESOURCE_TYPES, TEST_RESOURCE_TYPE, "persistedQueries.suffix", ""); - String expectedMetricPrefix = "org.apache.sling.graphql.core.servlet.GraphQLServlet.rt:" + TEST_RESOURCE_TYPE + ".m:GET.e:gql"; + String expectedMetricPrefix = "org.apache.sling.graphql.core.GraphQLServlet.rt:" + TEST_RESOURCE_TYPE + ".m:GET.e:gql"; verify(metricsService).counter(expectedMetricPrefix + ".cache_hits"); verify(metricsService).counter(expectedMetricPrefix + ".requests_total"); @@ -163,7 +163,7 @@ public class GraphQLServletTest { "persistedQueries.suffix", "/persisted"); // test resource type, default method, default extension - String expectedMetric = "org.apache.sling.graphql.core.servlet.GraphQLServlet.rt:" + TEST_RESOURCE_TYPE + ".m:GET.e:gql.cache_hit_rate"; + String expectedMetric = "org.apache.sling.graphql.core.GraphQLServlet.rt:" + TEST_RESOURCE_TYPE + ".m:GET.e:gql.cache_hit_rate"; assertTrue(metricRegistry.getGauges().containsKey(expectedMetric)); assertEquals(0.0f, metricRegistry.getGauges().get(expectedMetric).getValue());
