From ec0edb00e075ef6f35f3c550504002db6ee93ccc Mon Sep 17 00:00:00 2001 From: alandavis Date: Tue, 16 Aug 2022 13:44:40 +0100 Subject: [PATCH] Improved logging about transformers on registry refresh --- .../base/registry/CustomTransformers.java | 28 +++++--- .../base/registry/TransformRegistry.java | 70 ++++++++++++++++++- .../base/TransformControllerTest.java | 2 +- .../transform/registry/TransformerType.java | 41 +++++++++++ 4 files changed, 130 insertions(+), 11 deletions(-) create mode 100644 model/src/main/java/org/alfresco/transform/registry/TransformerType.java diff --git a/engines/base/src/main/java/org/alfresco/transform/base/registry/CustomTransformers.java b/engines/base/src/main/java/org/alfresco/transform/base/registry/CustomTransformers.java index be1c35af..36e951cf 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/registry/CustomTransformers.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/registry/CustomTransformers.java @@ -33,11 +33,11 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import javax.annotation.PostConstruct; -import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; @Component public class CustomTransformers @@ -57,13 +57,20 @@ public class CustomTransformers customTransformers.forEach(customTransformer -> customTransformersByName.put(customTransformer.getTransformerName(), customTransformer)); - logger.info("Transformers:"); - customTransformers.stream() - .sorted(Comparator.comparing(CustomTransformer::getTransformerName)) - .map(customTransformer -> customTransformer.getTransformerName()) - .filter(Objects::nonNull) - .map(name -> " "+name) - .forEach(logger::info); + List nonNullTransformerNames = customTransformers.stream() + .map(CustomTransformer::getTransformerName) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + + if (!nonNullTransformerNames.isEmpty()) + { + logger.info("Custom Transformers:"); + nonNullTransformerNames + .stream() + .sorted() + .map(name -> " "+name) + .forEach(logger::debug); + } } } @@ -77,4 +84,9 @@ public class CustomTransformers { customTransformersByName.put(name, customTransformer); } + + public List toList() + { + return customTransformers; + } } diff --git a/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java b/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java index e6830e35..99f76ca3 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java @@ -25,6 +25,7 @@ */ package org.alfresco.transform.base.registry; +import org.alfresco.transform.base.CustomTransformer; import org.alfresco.transform.config.TransformConfig; import org.alfresco.transform.config.TransformOption; import org.alfresco.transform.config.TransformOptionGroup; @@ -34,6 +35,7 @@ import org.alfresco.transform.registry.AbstractTransformRegistry; import org.alfresco.transform.registry.CombinedTransformConfig; import org.alfresco.transform.registry.Origin; import org.alfresco.transform.registry.TransformCache; +import org.alfresco.transform.registry.TransformerType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -48,7 +50,9 @@ import org.springframework.scheduling.annotation.Async; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Service; +import java.util.Collections; import java.util.Comparator; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -58,6 +62,7 @@ import java.util.Set; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Supplier; +import java.util.stream.Collectors; import java.util.stream.Stream; import static java.util.Collections.emptyMap; @@ -65,6 +70,8 @@ import static java.util.Objects.isNull; import static java.util.stream.Collectors.toUnmodifiableMap; import static java.util.stream.Collectors.toUnmodifiableSet; import static org.alfresco.transform.config.CoreVersionDecorator.setCoreVersionOnSingleStepTransformers; +import static org.alfresco.transform.registry.TransformerType.FAILOVER_TRANSFORMER; +import static org.alfresco.transform.registry.TransformerType.PIPELINE_TRANSFORMER; import static org.springframework.util.CollectionUtils.isEmpty; @Service @@ -79,6 +86,9 @@ public class TransformRegistry extends AbstractTransformRegistry @Value("${container.isTRouter}") private boolean isTRouter; + // Not autowired - avoids a circular reference in the router - initialised on startup event + private List customTransformers; + private static class Data extends TransformCache { private TransformConfig transformConfig; @@ -126,7 +136,7 @@ public class TransformRegistry extends AbstractTransformRegistry { final ApplicationContext context = event.getApplicationContext(); // the local "initEngineConfigs" method has to be called through the Spring proxy - context.getBean(TransformRegistry.class).initRegistryOnAppStartup(null); + context.getBean(TransformRegistry.class).initRegistryOnAppStartup(event); } /** @@ -139,12 +149,14 @@ public class TransformRegistry extends AbstractTransformRegistry backoff = @Backoff(delayExpression = "#{${transform.engine.config.retry.timeout} * 1000}")) void initRegistryOnAppStartup(final ContextRefreshedEvent event) { + customTransformers = event.getApplicationContext().getBean(CustomTransformers.class).toList(); retrieveConfig(); } /** * Recovery method in case all the retries fail. If not specified, the @Retryable method will cause the application - * to stop. + * to stop, which we don't want as the t-engine issue may have been sorted out in an hour when the next scheduled + * try is made. */ @Recover void recover(IllegalStateException e) @@ -180,6 +192,60 @@ public class TransformRegistry extends AbstractTransformRegistry TransformConfig transformConfig = combinedTransformConfig.buildTransformConfig(); Map> transformerByNameMap = combinedTransformConfig.getTransformerByNameMap(); concurrentUpdate(combinedTransformConfig, uncombinedTransformConfig, transformConfig, transformerByNameMap); + + logTransformers(uncombinedTransformConfig, combinedTransformConfig, transformerByNameMap); + } + + private void logTransformers(TransformConfig uncombinedTransformConfig, CombinedTransformConfig combinedTransformConfig, + Map> transformerByNameMap) + { + if (logger.isInfoEnabled()) + { + Set customTransformerNames = new HashSet(customTransformers == null + ? Collections.emptySet() + : customTransformers.stream().map(CustomTransformer::getTransformerName).collect(Collectors.toSet())); + List nonNullTransformerNames = uncombinedTransformConfig.getTransformers().stream() + .map(Transformer::getTransformerName) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + if (!nonNullTransformerNames.isEmpty()) + { + logger.info("Transformers (" + nonNullTransformerNames.size() + "):"); + nonNullTransformerNames + .stream() + .sorted() + .map(name -> { + Origin transformerOrigin = transformerByNameMap.get(name); + String message = " " + name + (transformerOrigin == null + ? " -- unavailable: missing transform steps" + : isTRouter + ? "" + : TransformerType.valueOf(transformerOrigin.get()) == PIPELINE_TRANSFORMER + ? " -- unavailable: pipeline only available via t-router" + : TransformerType.valueOf(transformerOrigin.get()) == FAILOVER_TRANSFORMER + ? " -- unavailable: failover only available via t-router" + : !customTransformerNames.contains(name) + ? " -- missing: CustomTransformer" + : ""); + customTransformerNames.remove(name); + return message; + }) + .forEach(logger::info); + + List unusedCustomTransformNames = customTransformerNames.stream() + .filter(Objects::nonNull) + .sorted() + .collect(Collectors.toList()); + if (!unusedCustomTransformNames.isEmpty()) + { + logger.info("Unused CustomTransformers (" + unusedCustomTransformNames.size() + " - name is not in the transform config):"); + unusedCustomTransformNames + .stream() + .map(name -> " " + name) + .forEach(logger::info); + } + } + } } public TransformConfig getTransformConfig() diff --git a/engines/base/src/test/java/org/alfresco/transform/base/TransformControllerTest.java b/engines/base/src/test/java/org/alfresco/transform/base/TransformControllerTest.java index d22a7ece..4643daa0 100644 --- a/engines/base/src/test/java/org/alfresco/transform/base/TransformControllerTest.java +++ b/engines/base/src/test/java/org/alfresco/transform/base/TransformControllerTest.java @@ -33,12 +33,12 @@ import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.AppenderBase; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; -import org.alfresco.transform.base.sfs.SharedFileStoreClient; import org.alfresco.transform.base.fakes.FakeTransformEngineWithTwoCustomTransformers; import org.alfresco.transform.base.fakes.FakeTransformerPdf2Png; import org.alfresco.transform.base.fakes.FakeTransformerTxT2Pdf; import org.alfresco.transform.base.model.FileRefEntity; import org.alfresco.transform.base.model.FileRefResponse; +import org.alfresco.transform.base.sfs.SharedFileStoreClient; import org.alfresco.transform.base.transform.TransformHandler; import org.alfresco.transform.client.model.TransformReply; import org.alfresco.transform.client.model.TransformRequest; diff --git a/model/src/main/java/org/alfresco/transform/registry/TransformerType.java b/model/src/main/java/org/alfresco/transform/registry/TransformerType.java new file mode 100644 index 00000000..f7f190bb --- /dev/null +++ b/model/src/main/java/org/alfresco/transform/registry/TransformerType.java @@ -0,0 +1,41 @@ +/* + * Copyright 2015-2022 Alfresco Software, Ltd. All rights reserved. + * + * License rights for this program may be obtained from Alfresco Software, Ltd. + * pursuant to a written agreement and any use of this program without such an + * agreement is prohibited. + */ +package org.alfresco.transform.registry; + +import org.alfresco.transform.config.Transformer; + +public enum TransformerType +{ + ENGINE_TRANSFORMER, + PIPELINE_TRANSFORMER, + FAILOVER_TRANSFORMER, + UNSUPPORTED_TRANSFORMER; + + public static TransformerType valueOf(Transformer transformer) + { + if (transformer == null) + { + return null; + } + if ((transformer.getTransformerFailover() == null || transformer.getTransformerFailover().isEmpty()) && + (transformer.getTransformerPipeline() == null || transformer.getTransformerPipeline().isEmpty())) + { + return ENGINE_TRANSFORMER; + } + if (transformer.getTransformerPipeline() != null && !transformer.getTransformerPipeline().isEmpty()) + { + return PIPELINE_TRANSFORMER; + } + if (transformer.getTransformerFailover() != null && !transformer.getTransformerFailover().isEmpty()) + { + return FAILOVER_TRANSFORMER; + } + + return UNSUPPORTED_TRANSFORMER; + } +} \ No newline at end of file