You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by re...@apache.org on 2024/09/11 16:27:23 UTC
(sling-org-apache-sling-resourceresolver) branch SLING-12402 created (now 3d27f0e)
This is an automated email from the ASF dual-hosted git repository.
reschke pushed a change to branch SLING-12402
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
at 3d27f0e SLING-12402: metrics for broken aliases (WIP)
This branch includes the following new commits:
new 3d27f0e SLING-12402: metrics for broken aliases (WIP)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(sling-org-apache-sling-resourceresolver) 01/01: SLING-12402: metrics for broken aliases (WIP)
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
reschke pushed a commit to branch SLING-12402
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit 3d27f0e75efb22b102c7a8dfdd8732b4e89e203a
Author: Julian Reschke <re...@apache.org>
AuthorDate: Wed Sep 11 17:27:01 2024 +0100
SLING-12402: metrics for broken aliases (WIP)
---
.../impl/ResourceResolverMetrics.java | 45 +++++++++++++++++-----
.../resourceresolver/impl/mapping/MapEntries.java | 11 +++++-
.../impl/mapping/MapEntriesTest.java | 13 ++++++-
3 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverMetrics.java b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverMetrics.java
index c073727..59fedf0 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverMetrics.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverMetrics.java
@@ -85,18 +85,27 @@ public class ResourceResolverMetrics {
private ServiceRegistration<Gauge<Long>> numberOfResourcesWithAliasesOnStartupGauge;
private Supplier<Long> numberOfResourcesWithAliasesOnStartupSupplier = ZERO_SUPPLIER;
- private Counter unclosedResourceResolvers;
+ // total number of detected invalid aliases on startup
+ private ServiceRegistration<Gauge<Long>> numberOfDetectedInvalidAliasesGauge;
+ private Supplier<Long> numberOfDetectedInvalidAliasesSupplier = ZERO_SUPPLIER;
+
+ // total number of detected conflicting aliases on startup
+ private ServiceRegistration<Gauge<Long>> numberOfDetectedConflictingAliasesGauge;
+ private Supplier<Long> numberOfDetectedConflictingAliasesSupplier = ZERO_SUPPLIER;
+ private Counter unclosedResourceResolvers;
@Activate
protected void activate(BundleContext bundleContext) {
- numberOfVanityPathsGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfVanityPaths", () -> numberOfVanityPathsSupplier );
- numberOfResourcesWithVanityPathsOnStartupGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfResourcesWithVanityPathsOnStartup", () -> numberOfResourcesWithVanityPathsOnStartupSupplier );
- numberOfVanityPathLookupsGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfVanityPathLookups", () -> numberOfVanityPathLookupsSupplier );
- numberOfVanityPathBloomNegativeGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfVanityPathBloomNegatives", () -> numberOfVanityPathBloomNegativeSupplier );
- numberOfVanityPathBloomFalsePositiveGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfVanityPathBloomFalsePositives", () -> numberOfVanityPathBloomFalsePositiveSupplier );
+ numberOfVanityPathsGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfVanityPaths", () -> numberOfVanityPathsSupplier);
+ numberOfResourcesWithVanityPathsOnStartupGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfResourcesWithVanityPathsOnStartup", () -> numberOfResourcesWithVanityPathsOnStartupSupplier);
+ numberOfVanityPathLookupsGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfVanityPathLookups", () -> numberOfVanityPathLookupsSupplier);
+ numberOfVanityPathBloomNegativeGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfVanityPathBloomNegatives", () -> numberOfVanityPathBloomNegativeSupplier);
+ numberOfVanityPathBloomFalsePositiveGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfVanityPathBloomFalsePositives", () -> numberOfVanityPathBloomFalsePositiveSupplier);
numberOfResourcesWithAliasedChildrenGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfResourcesWithAliasedChildren", () -> numberOfResourcesWithAliasedChildrenSupplier);
- numberOfResourcesWithAliasesOnStartupGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfResourcesWithAliasesOnStartup", () -> numberOfResourcesWithAliasesOnStartupSupplier );
+ numberOfResourcesWithAliasesOnStartupGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfResourcesWithAliasesOnStartup", () -> numberOfResourcesWithAliasesOnStartupSupplier);
+ numberOfDetectedInvalidAliasesGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfDetectedInvalidAliases", () -> numberOfDetectedInvalidAliasesSupplier);
+ numberOfDetectedConflictingAliasesGauge = registerGauge(bundleContext, METRICS_PREFIX + ".numberOfDetectedConflictingAliases", () -> numberOfDetectedConflictingAliasesSupplier);
unclosedResourceResolvers = metricsService.counter(METRICS_PREFIX + ".unclosedResourceResolvers");
}
@@ -107,8 +116,10 @@ public class ResourceResolverMetrics {
numberOfVanityPathLookupsGauge.unregister();
numberOfVanityPathBloomNegativeGauge.unregister();
numberOfVanityPathBloomFalsePositiveGauge.unregister();
- numberOfResourcesWithAliasesOnStartupGauge.unregister();
numberOfResourcesWithAliasedChildrenGauge.unregister();
+ numberOfResourcesWithAliasesOnStartupGauge.unregister();
+ numberOfDetectedInvalidAliasesGauge.unregister();
+ numberOfDetectedConflictingAliasesGauge.unregister();
}
/**
@@ -167,13 +178,29 @@ public class ResourceResolverMetrics {
numberOfResourcesWithAliasesOnStartupSupplier = supplier;
}
+ /**
+ * Set the supplier for the number of invalid aliases
+ * @param supplier a supplier returning the number of invalid aliases
+ */
+ public void setNumberOfDetectedInvalidAliasesSupplier(Supplier<Long> supplier) {
+ numberOfDetectedInvalidAliasesSupplier = supplier;
+ }
+
+ /**
+ * Set the supplier for the number of duplicate aliases
+ * @param supplier a supplier returning the number of conflicting aliases
+ */
+ public void setNumberOfDetectedConflictingAliasesSupplier(Supplier<Long> supplier) {
+ numberOfDetectedConflictingAliasesSupplier = supplier;
+ }
+
/**
* Increment the counter for the number of unresolved resource resolvers
*/
public void reportUnclosedResourceResolver() {
unclosedResourceResolvers.increment();
}
-
+
/**
* Create a gauge metrics.
*
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
index 44b83df..94b1111 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
@@ -146,6 +146,8 @@ public class MapEntries implements
private Map<String, Map<String, Collection<String>>> aliasMapsMap;
private final AtomicLong aliasResourcesOnStartup;
+ private final AtomicLong detectedConflictingAliases;
+ private final AtomicLong detectedInvalidAliases;
private final ReentrantLock initializing = new ReentrantLock();
@@ -180,6 +182,8 @@ public class MapEntries implements
this.stringInterpolationProvider = stringInterpolationProvider;
this.aliasResourcesOnStartup = new AtomicLong(0);
+ this.detectedConflictingAliases = new AtomicLong(0);
+ this.detectedInvalidAliases = new AtomicLong(0);
this.useOptimizeAliasResolution = doInit();
@@ -190,6 +194,7 @@ public class MapEntries implements
this.vanityPathLookups = new AtomicLong(0);
this.vanityPathBloomNegative = new AtomicLong(0);
this.vanityPathBloomFalsePositive = new AtomicLong(0);
+
initializeVanityPaths();
this.metrics = metrics;
@@ -201,6 +206,9 @@ public class MapEntries implements
this.metrics.get().setNumberOfVanityPathBloomFalsePositiveSupplier(vanityPathBloomFalsePositive::get);
this.metrics.get().setNumberOfResourcesWithAliasedChildrenSupplier(() -> (long) aliasMapsMap.size());
this.metrics.get().setNumberOfResourcesWithAliasesOnStartupSupplier(aliasResourcesOnStartup::get);
+ this.metrics.get().setNumberOfDetectedConflictingAliasesSupplier(detectedConflictingAliases::get);
+ this.metrics.get().setNumberOfDetectedInvalidAliasesSupplier(detectedInvalidAliases::get);
+ this.metrics.get().setNumberOfResourcesWithAliasesOnStartupSupplier(detectedInvalidAliases::get);
}
}
@@ -1266,7 +1274,8 @@ public class MapEntries implements
// the order matters here, the first alias in the array must come first
for (final String alias : aliasArray) {
if (isAliasInvalid(alias)) {
- log.warn("Encountered invalid alias '{}' under parent path '{}'. Refusing to use it.", alias, parentPath);
+ long invalids = detectedInvalidAliases.incrementAndGet();
+ log.warn("Encountered invalid alias '{}' under parent path '{}' (total {}). Refusing to use it.", alias, parentPath, invalids);
} else {
Map<String, Collection<String>> parentMap = map.computeIfAbsent(parentPath, key -> new ConcurrentHashMap<>());
Optional<String> siblingResourceNameWithDuplicateAlias = parentMap.entrySet().stream()
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
index 4402741..dce8d3a 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
@@ -100,6 +100,8 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
private EventAdmin eventAdmin;
private Map<String, Map<String, String>> aliasMap;
+ private AtomicLong detectedInvalidAliases;
+
private int testSize = 5;
private int pageSize;
@@ -162,10 +164,14 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
Optional<ResourceResolverMetrics> metrics = Optional.empty();
mapEntries = new MapEntries(resourceResolverFactory, bundleContext, eventAdmin, stringInterpolationProvider, metrics);
+
final Field aliasMapField = MapEntries.class.getDeclaredField("aliasMapsMap");
aliasMapField.setAccessible(true);
+ this.aliasMap = (Map<String, Map<String, String>>) aliasMapField.get(mapEntries);
- this.aliasMap = ( Map<String, Map<String, String>>) aliasMapField.get(mapEntries);
+ final Field detectedInvalidAliasesField = MapEntries.class.getDeclaredField("detectedInvalidAliases");
+ detectedInvalidAliasesField.setAccessible(true);
+ this.detectedInvalidAliases = (AtomicLong) detectedInvalidAliasesField.get(mapEntries);
}
@Override
@@ -232,16 +238,19 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
assertNotNull(aliasMap);
assertTrue(aliasMap.containsKey("child"));
assertEquals(List.of("foo", "bar", "qux", " "), aliasMap.get("child"));
+ assertEquals(3, detectedInvalidAliases.get());
}
@Test
public void test_alias_support_invalid() {
- for (String invalidAlias : List.of(".", "..", "foo/bar", "# foo", "")) {
+ List<String> invalidAliases = List.of(".", "..", "foo/bar", "# foo", "");
+ for (String invalidAlias : invalidAliases) {
prepareMapEntriesForAlias(false, false, invalidAlias);
mapEntries.doInit();
Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent");
assertEquals(Collections.emptyMap(), aliasMap);
}
+ assertEquals(invalidAliases.size(), detectedInvalidAliases.get());
}
private void prepareMapEntriesForAlias(boolean onJcrContent, boolean withNullParent, String... alias) {