You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cs...@apache.org on 2025/03/11 10:19:50 UTC
(sling-org-apache-sling-distribution-journal) 01/01: SLING-12706 - Add metrics for package installation
This is an automated email from the ASF dual-hosted git repository.
cschneider pushed a commit to branch SLING-12706
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-distribution-journal.git
commit 00cd659b94c6cc7c3f49971a8759d8f8e351b9e3
Author: Christian Schneider <cs...@adobe.com>
AuthorDate: Tue Mar 11 11:19:29 2025 +0100
SLING-12706 - Add metrics for package installation
---
.../journal/bookkeeper/BookKeeperFactory.java | 1 +
.../bookkeeper/ContentPackageExtractor.java | 16 +++++++++--
.../journal/bookkeeper/ErrorListener.java | 10 +++++--
.../journal/bookkeeper/PackageHandler.java | 6 +++--
.../journal/bookkeeper/SubscriberMetrics.java | 23 +++++++++++++++-
.../impl/subscriber/SubscriberConfiguration.java | 1 +
.../bookkeeper/ContentPackageExtractorTest.java | 31 +++++++++++++---------
.../journal/bookkeeper/ErrorListenerTest.java | 10 ++++++-
8 files changed, 78 insertions(+), 20 deletions(-)
diff --git a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeperFactory.java b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeperFactory.java
index cd4c28c..18c5090 100644
--- a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeperFactory.java
+++ b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/BookKeeperFactory.java
@@ -65,6 +65,7 @@ public class BookKeeperFactory {
) {
ContentPackageExtractor extractor = new ContentPackageExtractor(
packaging,
+ subscriberMetrics,
config.getPackageHandling(),
config.shouldExtractorOverwriteFolderPrimaryTypes());
PackageHandler packageHandler = new PackageHandler(packageBuilder, extractor, binaryStore);
diff --git a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/ContentPackageExtractor.java b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/ContentPackageExtractor.java
index 5f1e3a9..c56981c 100644
--- a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/ContentPackageExtractor.java
+++ b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/ContentPackageExtractor.java
@@ -24,6 +24,7 @@ import static org.apache.jackrabbit.vault.fs.api.IdConflictPolicy.LEGACY;
import java.io.IOException;
import java.util.List;
+import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;
import javax.jcr.Node;
@@ -31,6 +32,7 @@ import javax.jcr.RepositoryException;
import javax.jcr.Session;
import javax.jcr.nodetype.NodeType;
+import org.apache.commons.lang3.time.StopWatch;
import org.apache.jackrabbit.vault.fs.io.ImportOptions;
import org.apache.jackrabbit.vault.packaging.JcrPackage;
import org.apache.jackrabbit.vault.packaging.JcrPackageManager;
@@ -53,14 +55,18 @@ class ContentPackageExtractor {
private final Logger log = LoggerFactory.getLogger(this.getClass());
private final Packaging packageService;
+ private final SubscriberMetrics subscriberMetrics;
+
private final PackageHandling packageHandling;
private final boolean overwritePrimaryTypesOfFolders;
public ContentPackageExtractor(
Packaging packageService,
+ SubscriberMetrics subscriberMetrics,
PackageHandling packageHandling,
boolean overwritePrimaryTypesOfFolders) {
this.packageService = packageService;
+ this.subscriberMetrics = subscriberMetrics;
this.packageHandling = packageHandling;
this.overwritePrimaryTypesOfFolders = overwritePrimaryTypesOfFolders;
}
@@ -103,10 +109,12 @@ class ContentPackageExtractor {
}
private void installPackage(String path, Node node) throws RepositoryException, PackageException, IOException {
- log.info("Content package received at {}. Starting import.\n", path);
+ log.info("Content package received at {}. Starting import", path);
+ StopWatch stopWatch = new StopWatch();
+ stopWatch.start();
Session session = node.getSession();
JcrPackageManager packMgr = packageService.getPackageManager(session);
- ErrorListener listener = new ErrorListener();
+ ErrorListener listener = new ErrorListener(subscriberMetrics);
try (JcrPackage pack = packMgr.open(node)) {
if (pack != null) {
installPackage(pack, listener);
@@ -114,6 +122,10 @@ class ContentPackageExtractor {
} catch (PackageException e) {
failed(listener.getLastErrorMessage(), e);
}
+ subscriberMetrics.getPackageInstallCount().increment();
+ long durationMS = stopWatch.getTime(TimeUnit.MILLISECONDS);
+ subscriberMetrics.getPackgeInstallDuration().update(durationMS, TimeUnit.MILLISECONDS);
+ log.info("Content package at {} installed in durationMS={}", path, durationMS);
}
private void installPackage(JcrPackage pack, ErrorListener listener) throws RepositoryException, PackageException, IOException {
diff --git a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/ErrorListener.java b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/ErrorListener.java
index 2ce7455..30552c7 100644
--- a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/ErrorListener.java
+++ b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/ErrorListener.java
@@ -27,8 +27,13 @@ import static java.lang.String.format;
public class ErrorListener implements ProgressTrackerListener {
private String errorMessage = null;
+ private SubscriberMetrics subscriberMetrics;
- /**
+ public ErrorListener(SubscriberMetrics subscriberMetrics) {
+ this.subscriberMetrics = subscriberMetrics;
+ }
+
+ /**
* @return the error message of the last FileVault error logged ; or
* {@code null} if no error message has been logged.
*/
@@ -39,11 +44,12 @@ public class ErrorListener implements ProgressTrackerListener {
@Override
public void onMessage(Mode mode, String action, String path) {
- // ignore
+ subscriberMetrics.getFVMessages().increment();
}
@Override
public void onError(Mode mode, String path, Exception e) {
+ subscriberMetrics.getFVErrors().increment();
errorMessage = message(path, e);
}
diff --git a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/PackageHandler.java b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/PackageHandler.java
index 0607862..9fb79da 100644
--- a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/PackageHandler.java
+++ b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/PackageHandler.java
@@ -46,8 +46,10 @@ class PackageHandler {
private final BinaryStore binaryStore;
- public PackageHandler(DistributionPackageBuilder packageBuilder, ContentPackageExtractor extractor,
- BinaryStore binaryStore) {
+ public PackageHandler(
+ DistributionPackageBuilder packageBuilder,
+ ContentPackageExtractor extractor,
+ BinaryStore binaryStore) {
this.packageBuilder = packageBuilder;
this.extractor = extractor;
this.binaryStore = binaryStore;
diff --git a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/SubscriberMetrics.java b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/SubscriberMetrics.java
index eb6a231..14699f7 100644
--- a/src/main/java/org/apache/sling/distribution/journal/bookkeeper/SubscriberMetrics.java
+++ b/src/main/java/org/apache/sling/distribution/journal/bookkeeper/SubscriberMetrics.java
@@ -74,14 +74,17 @@ public class SubscriberMetrics {
// Counts every package that fails more than n times an thus causes a blocked queue
private static final String BLOCKING_IMPORT_ERRORS = SUB_COMPONENT + "import_errors";
+
private static final String IMPORT_PRE_PROCESS_REQUEST_COUNT = SUB_COMPONENT + "import_pre_process_request_count";
private static final String IMPORT_POST_PROCESS_SUCCESS_COUNT = SUB_COMPONENT + "import_post_process_success_count";
private static final String IMPORT_POST_PROCESS_REQUEST_COUNT = SUB_COMPONENT + "import_post_process_request_count";
private static final String INVALIDATION_PROCESS_SUCCESS_COUNT = SUB_COMPONENT + "invalidation_process_success_count";
private static final String INVALIDATION_PROCESS_REQUEST_COUNT = SUB_COMPONENT + "invalidation_process_request_count";
private static final String IMPORT_PRE_PROCESS_SUCCESS_COUNT = SUB_COMPONENT + "import_pre_process_success_count";
-
+
private static final String IMPORTED_PACKAGE_DURATION = SUB_COMPONENT + "imported_package_duration";
+ private static final String IMPORT_PACKAGE_INSTALL_DURATION = SUB_COMPONENT + "package_install_duration";
+ private static final String IMPORT_PACKAGE_INSTALL_COUNT = SUB_COMPONENT + "package_install_count";
private static final String REMOVED_PACKAGE_DURATION = SUB_COMPONENT + "removed_package_duration";
private static final String REMOVED_FAILED_PACKAGE_DURATION = SUB_COMPONENT + "removed_failed_package_duration";
private static final String SEND_STORED_STATUS_DURATION = SUB_COMPONENT + "send_stored_status_duration";
@@ -93,6 +96,8 @@ public class SubscriberMetrics {
private static final String INVALIDATION_PROCESS_DURATION = SUB_COMPONENT + "invalidation_process_duration";
private static final String CURRENT_IMPORT_DURATION = SUB_COMPONENT + "current_import_duration";
+ private static final String FV_MESSAGE_COUNT = SUB_COMPONENT + "fv_message_count";
+ private static final String FV_ERROR_COUNT = SUB_COMPONENT + "fv_error_count";
private final MetricsService metricsService;
private final Tag tagSubName;
@@ -243,6 +248,14 @@ public class SubscriberMetrics {
public Counter getImportPostProcessRequest() {
return metricsService.counter(getMetricName(IMPORT_POST_PROCESS_REQUEST_COUNT, tags));
}
+
+ public Timer getPackgeInstallDuration() {
+ return metricsService.timer(getMetricName(IMPORT_PACKAGE_INSTALL_DURATION, tags));
+ }
+
+ public Counter getPackageInstallCount() {
+ return metricsService.counter(getMetricName(IMPORT_PACKAGE_INSTALL_COUNT, tags));
+ }
public Timer getInvalidationProcessDuration() {
return metricsService.timer(getMetricName(INVALIDATION_PROCESS_DURATION, tags));
@@ -267,6 +280,14 @@ public class SubscriberMetrics {
public Counter getBlockingImportErrors() {
return metricsService.counter(getMetricName(BLOCKING_IMPORT_ERRORS, tags));
}
+
+ public Counter getFVMessages() {
+ return metricsService.counter(getMetricName(FV_MESSAGE_COUNT, tags));
+ }
+
+ public Counter getFVErrors() {
+ return metricsService.counter(getMetricName(FV_ERROR_COUNT, tags));
+ }
public void currentRetries(Supplier<Integer> retriesCallback) {
metricsService.gauge(getMetricName(CURRENT_RETRIES, tags), retriesCallback);
diff --git a/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/SubscriberConfiguration.java b/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/SubscriberConfiguration.java
index e4adc70..116083f 100644
--- a/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/SubscriberConfiguration.java
+++ b/src/main/java/org/apache/sling/distribution/journal/impl/subscriber/SubscriberConfiguration.java
@@ -67,4 +67,5 @@ public @interface SubscriberConfiguration {
@AttributeDefinition(description = "Number of ms to force subscriber reporting idle.")
int forceReadyMillies() default 300 * 1000;
+
}
diff --git a/src/test/java/org/apache/sling/distribution/journal/bookkeeper/ContentPackageExtractorTest.java b/src/test/java/org/apache/sling/distribution/journal/bookkeeper/ContentPackageExtractorTest.java
index 1888cf7..fe78092 100644
--- a/src/test/java/org/apache/sling/distribution/journal/bookkeeper/ContentPackageExtractorTest.java
+++ b/src/test/java/org/apache/sling/distribution/journal/bookkeeper/ContentPackageExtractorTest.java
@@ -44,7 +44,10 @@ import org.apache.jackrabbit.vault.packaging.Packaging;
import org.apache.sling.api.resource.PersistenceException;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.commons.metrics.MetricsService;
+import org.apache.sling.commons.metrics.internal.MetricsServiceImpl;
import org.apache.sling.distribution.common.DistributionException;
+import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
import org.apache.sling.testing.mock.sling.ResourceResolverType;
import org.apache.sling.testing.mock.sling.junit.SlingContext;
import org.junit.Before;
@@ -71,10 +74,14 @@ public class ContentPackageExtractorTest {
@Mock
private JcrPackage pkg;
+ private OsgiContext context = new OsgiContext();
private ResourceResolver resourceResolver;
+ private SubscriberMetrics subscriberMetrics;
@Before
public void before() throws RepositoryException {
+ MetricsService metricsService = context.registerInjectActivateService(MetricsServiceImpl.class);
+ subscriberMetrics = new SubscriberMetrics(metricsService, "publish_subscriber", "publish", false);
resourceResolver = scontext.resourceResolver();
when(packaging.getPackageManager(Mockito.any(Session.class)))
.thenReturn(packageManager);
@@ -87,7 +94,7 @@ public class ContentPackageExtractorTest {
Resource node = createNode(root, "other", NodeType.NT_FILE);
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Extract, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Extract, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
extractor.handle(resourceResolver, singletonList(node.getPath()));
verify(pkg, Mockito.never()).extract(Mockito.any(ImportOptions.class));
@@ -99,7 +106,7 @@ public class ContentPackageExtractorTest {
Resource node = createNode(packages, "mypackage", NodeType.NT_UNSTRUCTURED);
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Extract, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Extract, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
extractor.handle(resourceResolver, singletonList(node.getPath()));
verify(pkg, Mockito.never()).extract(Mockito.any(ImportOptions.class));
@@ -111,7 +118,7 @@ public class ContentPackageExtractorTest {
Resource packages = createEtcPackages();
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Extract, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Extract, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
// Should log a warning but not any exception
extractor.handle(resourceResolver, singletonList(packages.getPath() + "/invalid"));
@@ -122,7 +129,7 @@ public class ContentPackageExtractorTest {
Resource node = createImportedPackage();
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Off, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Off, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
extractor.handle(resourceResolver, singletonList(node.getPath()));
verify(pkg, Mockito.never()).extract(Mockito.any(ImportOptions.class));
@@ -134,7 +141,7 @@ public class ContentPackageExtractorTest {
Resource node = createImportedPackage();
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Extract, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Extract, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
extractor.handle(resourceResolver, singletonList(node.getPath()));
verify(pkg).extract(Mockito.any(ImportOptions.class));
@@ -145,7 +152,7 @@ public class ContentPackageExtractorTest {
Resource node = createImportedPackage();
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
extractor.handle(resourceResolver, singletonList(node.getPath()));
verify(pkg).install(Mockito.any(ImportOptions.class));
@@ -159,7 +166,7 @@ public class ContentPackageExtractorTest {
Resource node = createImportedPackage();
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
extractor.handle(resourceResolver, singletonList(node.getPath()));
}
@@ -177,7 +184,7 @@ public class ContentPackageExtractorTest {
Resource node = createImportedPackage();
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
extractor.handle(resourceResolver, singletonList(node.getPath()));
}
@@ -188,7 +195,7 @@ public class ContentPackageExtractorTest {
Resource node = createImportedPackage(packageRoot);
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
extractor.handle(resourceResolver, singletonList(node.getPath()));
verify(pkg, never()).install(Mockito.any(ImportOptions.class));
@@ -200,7 +207,7 @@ public class ContentPackageExtractorTest {
Resource node = createImportedPackage(packageRoot, NodeType.NT_FOLDER);
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
extractor.handle(resourceResolver, singletonList(node.getPath()));
verify(pkg, never()).install(Mockito.any(ImportOptions.class));
@@ -209,7 +216,7 @@ public class ContentPackageExtractorTest {
@Test
public void testNullPath() throws Exception {
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
extractor.handle(resourceResolver, singletonList(null));
verify(pkg, never()).install(Mockito.any(ImportOptions.class));
}
@@ -217,7 +224,7 @@ public class ContentPackageExtractorTest {
@Test
public void testNullPackageNode() throws Exception {
ContentPackageExtractor extractor = new ContentPackageExtractor(
- packaging, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
+ packaging, subscriberMetrics, PackageHandling.Install, OVERWRITE_PRIMARY_TYPES_OF_FOLDERS);
extractor.handle(resourceResolver, singletonList("/does/not/exist"));
verify(pkg, never()).install(Mockito.any(ImportOptions.class));
}
diff --git a/src/test/java/org/apache/sling/distribution/journal/bookkeeper/ErrorListenerTest.java b/src/test/java/org/apache/sling/distribution/journal/bookkeeper/ErrorListenerTest.java
index 33b0427..b5167d3 100644
--- a/src/test/java/org/apache/sling/distribution/journal/bookkeeper/ErrorListenerTest.java
+++ b/src/test/java/org/apache/sling/distribution/journal/bookkeeper/ErrorListenerTest.java
@@ -26,6 +26,10 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
+import org.apache.sling.commons.metrics.MetricsService;
+import org.apache.sling.commons.metrics.internal.MetricsServiceImpl;
+import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
+
public class ErrorListenerTest {
private ErrorListener errorListener;
@@ -35,10 +39,14 @@ public class ErrorListenerTest {
private static final String errorMsg = "Failed to import XYZ";
private static final Exception exception = new Exception(errorMsg);
+
+ private OsgiContext context = new OsgiContext();
@Before
public void before() {
- errorListener = new ErrorListener();
+ MetricsService metricsService = context.registerInjectActivateService(MetricsServiceImpl.class);
+ SubscriberMetrics subscriberMetrics = new SubscriberMetrics(metricsService, "publish_subscriber", "publish", false);
+ errorListener = new ErrorListener(subscriberMetrics);
}
@Test