You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2016/07/12 11:22:41 UTC
[1/2] brooklyn-server git commit: BROOKLYN-313: log.debug exceptions
thrown by effectors
Repository: brooklyn-server
Updated Branches:
refs/heads/master d64bef302 -> 7234e2b1b
BROOKLYN-313: log.debug exceptions thrown by effectors
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/b7cd606e
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/b7cd606e
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/b7cd606e
Branch: refs/heads/master
Commit: b7cd606e93c25340ddfc869df8c2b2e489b1120e
Parents: c3dc6e8
Author: Aled Sage <al...@gmail.com>
Authored: Sat Jul 9 18:41:29 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Sat Jul 9 22:16:35 2016 +0100
----------------------------------------------------------------------
.../brooklyn/core/effector/EffectorTasks.java | 2 +-
.../core/mgmt/internal/EffectorUtils.java | 7 +-
.../effector/EffectorExceptionLoggedTest.java | 178 +++++++++++++++++++
test-support/pom.xml | 4 +
.../org/apache/brooklyn/test/LogWatcher.java | 145 +++++++++++++++
5 files changed, 331 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/b7cd606e/core/src/main/java/org/apache/brooklyn/core/effector/EffectorTasks.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/effector/EffectorTasks.java b/core/src/main/java/org/apache/brooklyn/core/effector/EffectorTasks.java
index 68d45a5..b9429a0 100644
--- a/core/src/main/java/org/apache/brooklyn/core/effector/EffectorTasks.java
+++ b/core/src/main/java/org/apache/brooklyn/core/effector/EffectorTasks.java
@@ -87,7 +87,7 @@ public class EffectorTasks {
}) {
@Override
public void handleException(Throwable throwable) throws Exception {
- EffectorUtils.handleEffectorException(entity, effector, throwable);
+ throw EffectorUtils.handleEffectorException(entity, effector, throwable);
}
});
return dst.get();
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/b7cd606e/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EffectorUtils.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EffectorUtils.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EffectorUtils.java
index d2a8424..1b298e3 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EffectorUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EffectorUtils.java
@@ -278,9 +278,7 @@ public class EffectorUtils {
mgmtSupport.getEntityChangeListener().onEffectorCompleted(eff);
}
} catch (Exception e) {
- handleEffectorException(entity, eff, e);
- // (won't return below)
- return null;
+ throw handleEffectorException(entity, eff, e);
}
}
@@ -319,11 +317,12 @@ public class EffectorUtils {
EffectorCallPropagatedRuntimeException result = new EffectorCallPropagatedRuntimeException(entity, effector, throwable);
log.warn(Exceptions.collapseText(result));
+ log.debug(makeMessage(entity, effector), throwable);
throw result;
}
}
- public static void handleEffectorException(Entity entity, Effector<?> effector, Throwable throwable) {
+ public static RuntimeException handleEffectorException(Entity entity, Effector<?> effector, Throwable throwable) {
throw EffectorCallPropagatedRuntimeException.propagate(entity, effector, throwable);
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/b7cd606e/core/src/test/java/org/apache/brooklyn/core/effector/EffectorExceptionLoggedTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/effector/EffectorExceptionLoggedTest.java b/core/src/test/java/org/apache/brooklyn/core/effector/EffectorExceptionLoggedTest.java
new file mode 100644
index 0000000..f3854f1
--- /dev/null
+++ b/core/src/test/java/org/apache/brooklyn/core/effector/EffectorExceptionLoggedTest.java
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.core.effector;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintWriter;
+import java.util.concurrent.Callable;
+
+import org.apache.brooklyn.api.effector.Effector;
+import org.apache.brooklyn.api.effector.ParameterType;
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.mgmt.Task;
+import org.apache.brooklyn.api.mgmt.entitlement.EntitlementClass;
+import org.apache.brooklyn.api.mgmt.entitlement.EntitlementContext;
+import org.apache.brooklyn.api.mgmt.entitlement.EntitlementManager;
+import org.apache.brooklyn.core.effector.EffectorTasks.EffectorTaskFactory;
+import org.apache.brooklyn.core.entity.AbstractEntity;
+import org.apache.brooklyn.core.entity.Entities;
+import org.apache.brooklyn.core.internal.BrooklynProperties;
+import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
+import org.apache.brooklyn.core.mgmt.entitlement.Entitlements;
+import org.apache.brooklyn.core.mgmt.entitlement.Entitlements.EntityAndItem;
+import org.apache.brooklyn.core.mgmt.entitlement.Entitlements.StringAndArgument;
+import org.apache.brooklyn.core.mgmt.internal.EffectorUtils;
+import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
+import org.apache.brooklyn.core.test.entity.TestEntity;
+import org.apache.brooklyn.test.LogWatcher;
+import org.apache.brooklyn.test.LogWatcher.EventPredicates;
+import org.apache.brooklyn.util.core.config.ConfigBag;
+import org.apache.brooklyn.util.core.task.Tasks;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
+
+import ch.qos.logback.classic.spi.ILoggingEvent;
+
+public class EffectorExceptionLoggedTest extends BrooklynAppUnitTestSupport {
+
+ private static final Logger LOG = LoggerFactory.getLogger(AbstractEntity.class);
+
+ private TestEntity entity;
+
+ public static class ThrowingEntitlementManager implements EntitlementManager {
+ @Override
+ public <T> boolean isEntitled(EntitlementContext context, EntitlementClass<T> entitlementClass, T entitlementClassArgument) {
+ String type = entitlementClass.entitlementClassIdentifier();
+ if (Entitlements.INVOKE_EFFECTOR.entitlementClassIdentifier().equals(type)) {
+ @SuppressWarnings("unchecked")
+ String effectorName = ((EntityAndItem<StringAndArgument>)entitlementClassArgument).getItem().getString();
+ if ("myEffector".equals(effectorName)) {
+ LOG.info("Simulating NPE in entitlement manager");
+ throw new NullPointerException();
+ }
+ }
+ return true;
+ }
+ }
+
+ protected BrooklynProperties getBrooklynProperties() {
+ BrooklynProperties result = BrooklynProperties.Factory.newEmpty();
+ result.put(Entitlements.GLOBAL_ENTITLEMENT_MANAGER, ThrowingEntitlementManager.class.getName());
+ return result;
+ }
+
+ @BeforeMethod(alwaysRun=true)
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+ entity = app.addChild(EntitySpec.create(TestEntity.class));
+ }
+
+ @Test
+ public void testInvokeEffectorDirectlyIncludesException() throws Exception {
+ try {
+ entity.myEffector();
+ } catch (Exception e) {
+ assertExceptionContainsIsEntitledStack(e);
+ }
+
+ try {
+ Entities.invokeEffector(app, entity, TestEntity.MY_EFFECTOR).get();
+ } catch (Exception e) {
+ assertExceptionContainsIsEntitledStack(e);
+ }
+ }
+
+ @Test
+ public void testInvokeViaOtherEffectorIncludesException() throws Exception {
+ EffectorTaskFactory<Void> body = new EffectorTaskFactory<Void>() {
+ @Override public Task<Void> newTask(final Entity entity, final Effector<Void> effector, final ConfigBag parameters) {
+ return Tasks.<Void>builder()
+ .body(new Callable<Void>() {
+ @Override public Void call() throws Exception {
+ EffectorExceptionLoggedTest.this.entity.myEffector();
+ return null;
+ }})
+ .build();
+ }
+ };
+ EffectorAndBody<Void> effector = new EffectorAndBody<Void>("callingEffector", Void.class, ImmutableList.<ParameterType<?>>of(), "my description", body);
+
+ try {
+ Entities.invokeEffector(app, app, effector).get();
+ } catch (Exception e) {
+ assertExceptionContainsIsEntitledStack(e);
+ }
+ }
+
+ /**
+ * This task-invocation pattern matches that used in AutoScaler.
+ *
+ * Confirm that we log the real stacktrace of the exception.
+ *
+ * This requires adding a mock appender, asserting it gets called with our desired
+ * class+method name, and then removing the appender!
+ */
+ @Test
+ public void testInvokeInTask() throws Exception {
+ String loggerName = EffectorUtils.class.getName();
+ ch.qos.logback.classic.Level logLevel = ch.qos.logback.classic.Level.DEBUG;
+ Predicate<ILoggingEvent> filter = Predicates.and(EventPredicates.containsMessage("Error invoking myEffector"),
+ EventPredicates.containsExceptionStackLine(ThrowingEntitlementManager.class, "isEntitled"));
+ LogWatcher watcher = new LogWatcher(loggerName, logLevel, filter);
+
+ watcher.start();
+ try {
+ Entities.submit(entity, Tasks.<Void>builder().displayName("Effector-invoker")
+ .description("Invoke in task")
+ .tag(BrooklynTaskTags.NON_TRANSIENT_TASK_TAG)
+ .body(new Callable<Void>() {
+ @Override
+ public Void call() throws Exception {
+ entity.myEffector();
+ return null;
+ }
+ }).build())
+ .blockUntilEnded();
+
+ watcher.assertHasEventEventually();
+ } finally {
+ watcher.close();
+ }
+ }
+
+ private void assertExceptionContainsIsEntitledStack(Exception e) throws Exception {
+ String expected = ThrowingEntitlementManager.class.getSimpleName()+".isEntitled";
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ PrintWriter writer = new PrintWriter(baos);
+ e.printStackTrace(writer);
+ writer.flush();
+ String eString = new String(baos.toByteArray());
+ if (!eString.contains(expected)) {
+ throw new Exception("Original exception does not contain '"+expected+"'", e);
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/b7cd606e/test-support/pom.xml
----------------------------------------------------------------------
diff --git a/test-support/pom.xml b/test-support/pom.xml
index 84beaf6..1cb977e 100644
--- a/test-support/pom.xml
+++ b/test-support/pom.xml
@@ -59,5 +59,9 @@
<groupId>org.testng</groupId>
<artifactId>testng</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ </dependency>
</dependencies>
</project>
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/b7cd606e/test-support/src/main/java/org/apache/brooklyn/test/LogWatcher.java
----------------------------------------------------------------------
diff --git a/test-support/src/main/java/org/apache/brooklyn/test/LogWatcher.java b/test-support/src/main/java/org/apache/brooklyn/test/LogWatcher.java
new file mode 100644
index 0000000..9326f1b
--- /dev/null
+++ b/test-support/src/main/java/org/apache/brooklyn/test/LogWatcher.java
@@ -0,0 +1,145 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.test;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+import static org.testng.Assert.assertFalse;
+
+import java.io.Closeable;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.mockito.Mockito;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.Beta;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.classic.spi.IThrowableProxy;
+import ch.qos.logback.classic.spi.StackTraceElementProxy;
+import ch.qos.logback.core.Appender;
+
+/**
+ * Testing utility that registers an appender to watch a given logback logger, and records events
+ * that match a given predicate.
+ *
+ * Callers should first call {@link #start()}, and must call {@link #close()} to de-register the
+ * appender (doing this in a finally block).
+ */
+@Beta
+public class LogWatcher implements Closeable {
+
+ private static final Logger LOG = LoggerFactory.getLogger(LogWatcher.class);
+
+ public static class EventPredicates {
+ public static Predicate<ILoggingEvent> containsMessage(final String expected) {
+ return new Predicate<ILoggingEvent>() {
+ @Override public boolean apply(ILoggingEvent input) {
+ if (input == null) return false;
+ String msg = input.getFormattedMessage();
+ return (msg != null) && msg.contains(expected);
+ }
+ };
+ }
+
+ public static Predicate<ILoggingEvent> containsExceptionStackLine(final Class<?> clazz, final String methodName) {
+ return new Predicate<ILoggingEvent>() {
+ @Override public boolean apply(ILoggingEvent input) {
+ IThrowableProxy throwable = (input != null) ? input.getThrowableProxy() : null;
+ if (throwable != null) {
+ for (StackTraceElementProxy line : throwable.getStackTraceElementProxyArray()) {
+ if (line.getStackTraceElement().getClassName().contains(clazz.getSimpleName())
+ && line.getStackTraceElement().getMethodName().contains(methodName)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+ };
+ }
+ }
+
+ private final List<ILoggingEvent> events = Collections.synchronizedList(Lists.<ILoggingEvent>newLinkedList());
+ private final AtomicBoolean closed = new AtomicBoolean();
+ private final ch.qos.logback.classic.Level loggerLevel;
+ private final ch.qos.logback.classic.Logger watchedLogger;
+ private final Appender<ILoggingEvent> appender;
+ private volatile Level origLevel;
+
+ @SuppressWarnings("unchecked")
+ public LogWatcher(String loggerName, ch.qos.logback.classic.Level loggerLevel, final Predicate<? super ILoggingEvent> filter) {
+ this.watchedLogger = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(checkNotNull(loggerName, "loggerName"));
+ this.loggerLevel = checkNotNull(loggerLevel, "loggerLevel");
+ this.appender = Mockito.mock(Appender.class);
+
+ Mockito.when(appender.getName()).thenReturn("MOCK");
+ Answer<Void> answer = new Answer<Void>() {
+ @Override
+ public Void answer(InvocationOnMock invocation) throws Throwable {
+ ILoggingEvent event = invocation.getArgumentAt(0, ILoggingEvent.class);
+ if (event != null && filter.apply(event)) {
+ events.add(event);
+ }
+ LOG.trace("level="+event.getLevel()+"; event="+event+"; msg="+event.getFormattedMessage());
+ return null;
+ }
+ };
+ Mockito.doAnswer(answer).when(appender).doAppend(Mockito.<ILoggingEvent>any());
+ }
+
+ public void start() {
+ checkState(!closed.get(), "Cannot start LogWatcher after closed");
+ origLevel = watchedLogger.getLevel();
+ watchedLogger.setLevel(loggerLevel);
+ watchedLogger.addAppender(appender);
+ }
+
+ @Override
+ public void close() {
+ if (closed.compareAndSet(false, true)) {
+ if (watchedLogger != null) {
+ if (origLevel != null) watchedLogger.setLevel(origLevel);
+ watchedLogger.detachAppender(appender);
+ }
+ }
+ }
+
+ public void assertHasEventEventually() {
+ Asserts.succeedsEventually(new Runnable() {
+ public void run() {
+ assertFalse(events.isEmpty());
+ }});
+ }
+
+ public List<ILoggingEvent> getEvents() {
+ synchronized (events) {
+ return ImmutableList.copyOf(events);
+ }
+ }
+}
[2/2] brooklyn-server git commit: This closes #245
Posted by al...@apache.org.
This closes #245
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/7234e2b1
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/7234e2b1
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/7234e2b1
Branch: refs/heads/master
Commit: 7234e2b1b118ada66de1748b37599a724c32ff6b
Parents: d64bef3 b7cd606
Author: Aled Sage <al...@gmail.com>
Authored: Tue Jul 12 12:22:18 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Tue Jul 12 12:22:18 2016 +0100
----------------------------------------------------------------------
.../brooklyn/core/effector/EffectorTasks.java | 2 +-
.../core/mgmt/internal/EffectorUtils.java | 7 +-
.../effector/EffectorExceptionLoggedTest.java | 178 +++++++++++++++++++
test-support/pom.xml | 4 +
.../org/apache/brooklyn/test/LogWatcher.java | 145 +++++++++++++++
5 files changed, 331 insertions(+), 5 deletions(-)
----------------------------------------------------------------------