This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit 9c5cc10af7520316b8fbfd630d8cdc48448d2581 Author: Alex Heneveld <[email protected]> AuthorDate: Thu Jan 6 16:43:21 2022 +0000 tidy logging esp for some edge case errors --- .../BrooklynComponentTemplateResolver.java | 20 ++++++----- .../catalog/internal/BasicBrooklynCatalog.java | 39 +++++++++++++--------- .../core/objs/BasicEntityTypeRegistry.java | 4 +++ .../core/typereg/TypePlanTransformers.java | 14 +++++--- 4 files changed, 48 insertions(+), 29 deletions(-) diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java index f8c9844..eacb289 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java @@ -199,15 +199,17 @@ public class BrooklynComponentTemplateResolver { } throw new IllegalStateException("Unable to create spec for type " + type + ". " + msgDetails); } - spec = EntityManagementUtils.unwrapEntity(spec); - - CampResolver.fixScopeRootAtRoot(mgmt, spec); - - populateSpec(spec, encounteredRegisteredTypeSymbolicNames); - - @SuppressWarnings("unchecked") - EntitySpec<T> typedSpec = (EntitySpec<T>) spec; - return typedSpec; + try { + spec = EntityManagementUtils.unwrapEntity(spec); + CampResolver.fixScopeRootAtRoot(mgmt, spec); + populateSpec(spec, encounteredRegisteredTypeSymbolicNames); + + @SuppressWarnings("unchecked") + EntitySpec<T> typedSpec = (EntitySpec<T>) spec; + return typedSpec; + } catch (Exception e) { + throw Exceptions.propagateAnnotated("Error populating spec "+spec, e); + } } private List<EntitySpecResolver> getServiceTypeResolverOverrides() { diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java index 4cde1de..64b578f 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java @@ -843,7 +843,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { // if version not set, infer from: id, then from name, then item version if (versionFromId!=null) { if (Strings.isNonBlank(version) && !versionFromId.equals(version)) { - throw new IllegalArgumentException("Discrepency between version set in id " + versionFromId + " and version property " + version); + throw new IllegalArgumentException("Discrepancy between version set in id " + versionFromId + " and version property " + version); } version = versionFromId; } @@ -857,8 +857,8 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { if (Strings.isBlank(version)) { version = setFromItemIfUnset(version, itemAsMap, "version"); version = setFromItemIfUnset(version, itemAsMap, "template_version"); - if (version==null) { - log.debug("No version specified for catalog item " + symbolicName + ". Using default value."); + if (Strings.isBlank(version)) { + if (log.isTraceEnabled()) log.trace("No version specified for catalog item " + symbolicName + " or BOM ancestors. Using default/bundle value."); version = null; } } @@ -958,6 +958,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { } if (version==null) { // use this as default version when nothing specified or inferrable from containing bundle + log.debug("No version specified for catalog item " + symbolicName + " or BOM ancestors and not available from bundle. Using default value "+BasicBrooklynCatalog.NO_VERSION+"."); version = BasicBrooklynCatalog.NO_VERSION; } } @@ -1605,7 +1606,8 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { * primarily used to downgrade log messages when trying to resolve with different strategies. * can also be used to say which item is being currently resolved. */ - public static ThreadLocal<String> currentlyResolvingType = new ThreadLocal<String>(); + public static ThreadLocal<String> currentlyResolvingType = new ThreadLocal<>(); + public static ThreadLocal<RegisteredType> currentlyValidatingType = new ThreadLocal<>(); private String makeAsIndentedList(String yaml) { String[] lines = yaml.split("\n"); @@ -1790,20 +1792,25 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { @Override @Beta public Collection<Throwable> validateType(RegisteredType typeToValidate, RegisteredTypeLoadingContext constraint, boolean allowUnresolved) { - ReferenceWithError<RegisteredType> result = validateResolve(typeToValidate, constraint); - if (result.hasError()) { - if (allowUnresolved && RegisteredTypes.isTemplate(typeToValidate)) { - // ignore for templates - return Collections.emptySet(); - } - if (result.getError() instanceof CompoundRuntimeException) { - return ((CompoundRuntimeException)result.getError()).getAllCauses(); + try { + currentlyValidatingType.set(typeToValidate); + ReferenceWithError<RegisteredType> result = validateResolve(typeToValidate, constraint); + if (result.hasError()) { + if (allowUnresolved && RegisteredTypes.isTemplate(typeToValidate)) { + // ignore for templates + return Collections.emptySet(); + } + if (result.getError() instanceof CompoundRuntimeException) { + return ((CompoundRuntimeException)result.getError()).getAllCauses(); + } + return Collections.singleton(result.getError()); } - return Collections.singleton(result.getError()); + // replace what's in catalog with resolved+validated version + ((BasicBrooklynTypeRegistry) mgmt.getTypeRegistry()).addToLocalUnpersistedTypeRegistry(result.get(), true); + return Collections.emptySet(); + } finally { + currentlyValidatingType.set(null); } - // replace what's in catalog with resolved+validated version - ((BasicBrooklynTypeRegistry) mgmt.getTypeRegistry()).addToLocalUnpersistedTypeRegistry(result.get(), true); - return Collections.emptySet(); } /** diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BasicEntityTypeRegistry.java b/core/src/main/java/org/apache/brooklyn/core/objs/BasicEntityTypeRegistry.java index 720241c..206f84d 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/BasicEntityTypeRegistry.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/BasicEntityTypeRegistry.java @@ -105,12 +105,16 @@ public class BasicEntityTypeRegistry implements EntityTypeRegistry { } private <T extends Entity> Class<? extends T> getFromAnnotation(Class<T> type) { + try { ImplementedBy annotation = type.getAnnotation(org.apache.brooklyn.api.entity.ImplementedBy.class); if (annotation == null) return null; Class<? extends Entity> value = annotation.value(); checkIsImplementation(type, value); return (Class<? extends T>) value; + } catch (Exception e) { + throw Exceptions.propagateAnnotated("Error reading ImplementedBy on "+type, e); + } } private <T extends Entity> Class<? super T> getInterfaceWithAnnotationMatching(Class<T> implClazz) { diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/TypePlanTransformers.java b/core/src/main/java/org/apache/brooklyn/core/typereg/TypePlanTransformers.java index 94d2b57..8de3d51 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/TypePlanTransformers.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/TypePlanTransformers.java @@ -138,14 +138,20 @@ public class TypePlanTransformers { } if (log.isDebugEnabled()) { - Supplier<String> s = () -> "Failure transforming plan; returning summary failure, but for reference " + Supplier<String> s = () -> "Interim failure transforming plan " + + BasicBrooklynCatalog.currentlyResolvingType.get()+"/"+BasicBrooklynCatalog.currentlyValidatingType.get() + + "; will throw/return summary failure and catalog routines often wrap and retry, " + + "but for reference in the event this failure is unexpected: " + "potentially applicable transformers were "+transformers+", " + "available ones are "+MutableList.builder().addAll(all(mgmt)).build()+"; " - + "failures: "+failuresFromTransformers +"; " - + "unsupported by: "+transformersWhoDontSupport; - if (BasicBrooklynCatalog.currentlyResolvingType.get()==null) { + + "unsupported by: "+transformersWhoDontSupport+"; " + + "failures: "+failuresFromTransformers + ; + if (BasicBrooklynCatalog.currentlyResolvingType.get()==null && BasicBrooklynCatalog.currentlyValidatingType.get()==null) { + // if both are null, we don't know who is calling us, so log it as debug log.debug(s.get()); } else if (log.isTraceEnabled()) { + // trace can be enabled to get more information if this is occurring unexpectedly log.trace(s.get()); } }
