diff --git a/tritium-brave/build.gradle b/tritium-brave/build.gradle index 5de4a6822..f51e41844 100644 --- a/tritium-brave/build.gradle +++ b/tritium-brave/build.gradle @@ -3,6 +3,7 @@ apply from: "${rootDir}/gradle/publish-jars.gradle" dependencies { compile project(':tritium-api') compile project(':tritium-core') + compile 'com.palantir.safe-logging:safe-logging' compile 'io.zipkin.brave:brave-core' compile 'org.slf4j:slf4j-api' diff --git a/tritium-brave/src/main/java/com/palantir/tritium/brave/BraveLocalTracingInvocationEventHandler.java b/tritium-brave/src/main/java/com/palantir/tritium/brave/BraveLocalTracingInvocationEventHandler.java index e977b2f0c..199449369 100644 --- a/tritium-brave/src/main/java/com/palantir/tritium/brave/BraveLocalTracingInvocationEventHandler.java +++ b/tritium-brave/src/main/java/com/palantir/tritium/brave/BraveLocalTracingInvocationEventHandler.java @@ -32,7 +32,7 @@ public final class BraveLocalTracingInvocationEventHandler extends AbstractInvocationEventHandler { - private static final Logger LOGGER = LoggerFactory.getLogger(BraveLocalTracingInvocationEventHandler.class); + private static final Logger logger = LoggerFactory.getLogger(BraveLocalTracingInvocationEventHandler.class); private final Brave brave; private final String component; @@ -71,7 +71,7 @@ public void onFailure(@Nullable InvocationContext context, @Nonnull Throwable ca private void debugIfNullContext(@Nullable InvocationContext context) { if (context == null) { - LOGGER.debug("Encountered null metric context likely due to exception in preInvocation"); + logger.debug("Encountered null metric context likely due to exception in preInvocation"); } } diff --git a/tritium-caffeine/build.gradle b/tritium-caffeine/build.gradle index 3726be837..d0974f052 100644 --- a/tritium-caffeine/build.gradle +++ b/tritium-caffeine/build.gradle @@ -10,6 +10,7 @@ dependencies { compile 'com.github.ben-manes.caffeine:caffeine' compile 'com.google.code.findbugs:annotations' compile 'com.google.guava:guava' + compile 'com.palantir.safe-logging:safe-logging' compile 'io.dropwizard.metrics:metrics-core' compile 'org.slf4j:slf4j-api' diff --git a/tritium-core/build.gradle b/tritium-core/build.gradle index e67715821..1abeecdeb 100644 --- a/tritium-core/build.gradle +++ b/tritium-core/build.gradle @@ -4,6 +4,7 @@ dependencies { compile project(':tritium-api') compile 'com.google.guava:guava' compile 'com.google.code.findbugs:annotations' + compile 'com.palantir.safe-logging:safe-logging' compile 'org.slf4j:slf4j-api' testCompile 'com.google.guava:guava-testlib' diff --git a/tritium-core/src/main/java/com/palantir/tritium/event/CompositeInvocationEventHandler.java b/tritium-core/src/main/java/com/palantir/tritium/event/CompositeInvocationEventHandler.java index 01012fcf2..9a1c3335a 100644 --- a/tritium-core/src/main/java/com/palantir/tritium/event/CompositeInvocationEventHandler.java +++ b/tritium-core/src/main/java/com/palantir/tritium/event/CompositeInvocationEventHandler.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.collect.ImmutableList; +import com.palantir.logsafe.SafeArg; import java.lang.reflect.Method; import java.util.Arrays; import java.util.List; @@ -29,7 +30,7 @@ public final class CompositeInvocationEventHandler extends AbstractInvocationEventHandler { - private static final Logger LOGGER = LoggerFactory.getLogger(CompositeInvocationEventHandler.class); + private static final Logger logger = LoggerFactory.getLogger(CompositeInvocationEventHandler.class); private final List> handlers; @@ -76,7 +77,7 @@ public void onSuccess(@Nullable InvocationContext invocationContext, @Nullable O } } } else { - LOGGER.debug("onSuccess InvocationContext was not a CompositeInvocationContext: {}", invocationContext); + logger.debug("onSuccess InvocationContext was not a CompositeInvocationContext: {}", invocationContext); } } @@ -93,7 +94,7 @@ public void onFailure(@Nullable InvocationContext invocationContext, @Nonnull Th } } } else { - LOGGER.debug("onFailure InvocationContext was not a CompositeInvocationContext: {}", invocationContext); + logger.debug("onFailure InvocationContext was not a CompositeInvocationContext: {}", invocationContext); } } @@ -118,11 +119,15 @@ public String toString() { private static void preInvocationFailed(InvocationEventHandler handler, Object instance, Method method, Object[] args, Exception exception) { - LOGGER.warn("Exception handling preInvocation({}): " + logger.warn("Exception handling preInvocation({}): " + "invocation of {}.{} with arguments {} on {} threw: {}", handler, - method.getDeclaringClass().getCanonicalName(), method.getName(), Arrays.toString(args), instance, - exception, exception); + SafeArg.of("class", method.getDeclaringClass().getCanonicalName()), + SafeArg.of("method", method.getName()), + Arrays.toString(args), + instance, + exception, + exception); } private void handleSuccess(InvocationEventHandler handler, @@ -132,7 +137,7 @@ private void handleSuccess(InvocationEventHandler handler, try { handler.onSuccess(context, result); } catch (RuntimeException e) { - LOGGER.warn("Exception handling onSuccess({}, {}): {}", + logger.warn("Exception handling onSuccess({}, {}): {}", context, result, e, e); } } @@ -144,7 +149,7 @@ private void handleFailure(InvocationEventHandler handler, try { handler.onFailure(context, cause); } catch (RuntimeException e) { - LOGGER.warn("Exception handling onFailure({}, {}): {}", + logger.warn("Exception handling onFailure({}, {}): {}", context, cause, e, e); } } diff --git a/tritium-lib/build.gradle b/tritium-lib/build.gradle index 676ad3044..c4b6aa437 100644 --- a/tritium-lib/build.gradle +++ b/tritium-lib/build.gradle @@ -7,6 +7,7 @@ dependencies { compile project(':tritium-slf4j') compile 'com.google.code.findbugs:annotations' compile 'com.google.guava:guava' + compile 'com.palantir.safe-logging:safe-logging' compile 'org.slf4j:slf4j-api' testCompile project(path: ':tritium-core', configuration: 'testArtifacts') diff --git a/tritium-lib/src/main/java/com/palantir/tritium/proxy/InvocationEventProxy.java b/tritium-lib/src/main/java/com/palantir/tritium/proxy/InvocationEventProxy.java index ea54503d5..59c70c7e9 100644 --- a/tritium-lib/src/main/java/com/palantir/tritium/proxy/InvocationEventProxy.java +++ b/tritium-lib/src/main/java/com/palantir/tritium/proxy/InvocationEventProxy.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.reflect.AbstractInvocationHandler; +import com.palantir.logsafe.SafeArg; import com.palantir.tritium.api.functions.BooleanSupplier; import com.palantir.tritium.event.CompositeInvocationEventHandler; import com.palantir.tritium.event.InstrumentationFilter; @@ -37,7 +38,7 @@ abstract class InvocationEventProxy extends AbstractInvocationHandler implements InvocationHandler { - private static final Logger LOGGER = LoggerFactory.getLogger(InvocationEventProxy.class); + private static final Logger logger = LoggerFactory.getLogger(InvocationEventProxy.class); private static final Object[] NO_ARGS = {}; private final InstrumentationFilter filter; @@ -45,6 +46,7 @@ abstract class InvocationEventProxy /** * Always enabled instrumentation handler. + * * @param handlers event handlers */ protected InvocationEventProxy(List> handlers) { @@ -81,12 +83,12 @@ private boolean isEnabled(Object instance, Method method, Object[] args) { return eventHandler.isEnabled() && filter.shouldInstrument(instance, method, args); } catch (Throwable t) { - LOGGER.warn("Exception handling isEnabled({}): {}", - toInvocationDebugString(instance, method, args), t, t); + logInvocationWarning("isEnabled", instance, method, args, t); return false; } } + @Override @Nullable @SuppressWarnings("checkstyle:illegalthrows") @@ -102,7 +104,7 @@ protected final Object handleInvocation(Object proxy, Method method, Object[] ar * {@link #invoke} delegates to this method upon any method invocation on the instance, except * {@link Object#equals}, {@link Object#hashCode} and {@link Object#toString}. The result will be returned as the * proxied method's return value. - * + *

*

* Unlike {@link #invoke}, {@code args} will never be null. When the method has no parameter, an empty array is * passed in. @@ -126,8 +128,7 @@ final InvocationContext handlePreInvocation(Object instance, Method method, Obje try { return eventHandler.preInvocation(instance, method, args); } catch (RuntimeException e) { - LOGGER.warn("Exception handling preInvocation({}): {}", - toInvocationDebugString(instance, method, args), e, e); + logInvocationWarning("preInvocation", instance, method, args, e); } return null; } @@ -147,30 +148,48 @@ final Object handleOnSuccess(@Nullable InvocationContext context, @Nullable Obje try { eventHandler.onSuccess(context, result); } catch (RuntimeException e) { - LOGGER.warn("Exception handling onSuccess({}, {}): {}", - context, result, e, e); + logInvocationWarningOnSuccess(context, result, e); } return result; } + private static void logInvocationWarningOnSuccess(InvocationContext context, Object result, Exception cause) { + logger.warn("{} exception handling onSuccess({}, {}): {}", + safeSimpleClassName("cause", cause), context, safeSimpleClassName("result", result), cause); + } + final Throwable handleOnFailure(@Nullable InvocationContext context, Throwable cause) { try { eventHandler.onFailure(context, cause); } catch (RuntimeException e) { - LOGGER.warn("Exception handling onFailure({}, {}): {}", - context, cause, e, e); + logInvocationWarningOnFailure(context, cause, e); } return cause; } - protected final String toInvocationDebugString(Object instance, Method method, @Nullable Object[] args) { - return "invocation of " - + method.getDeclaringClass() + '.' + method.getName() - + " with arguments " + Arrays.toString(nullToEmpty(args)) - + " on " + instance + " via " + this + " : "; + private static void logInvocationWarningOnFailure(InvocationContext context, @Nullable Throwable result, + RuntimeException cause) { + logger.warn("{} exception handling onFailure({}, {}): {}", + safeSimpleClassName("cause", cause), context, safeSimpleClassName("result", result), cause); + } + + private static SafeArg safeSimpleClassName(String name, @Nullable Object object) { + return SafeArg.of(name, (object == null) ? "null" : object.getClass().getSimpleName()); + } + + static void logInvocationWarning(String event, Object instance, Method method, Object[] args, + Throwable cause) { + logger.warn("{} exception handling {} invocation of {}.{} with arguments {} on {}", + safeSimpleClassName("throwable", cause), + event, + SafeArg.of("class", method.getDeclaringClass()), + SafeArg.of("method", method.getName()), + Arrays.toString(nullToEmpty(args)), + instance, + cause); } - protected static Object[] nullToEmpty(@Nullable Object[] args) { + private static Object[] nullToEmpty(@Nullable Object[] args) { return (args == null) ? NO_ARGS : args; } diff --git a/tritium-lib/src/test/java/com/palantir/tritium/proxy/InvocationEventProxyTest.java b/tritium-lib/src/test/java/com/palantir/tritium/proxy/InvocationEventProxyTest.java index 6046fe2b9..bb3abb6e3 100644 --- a/tritium-lib/src/test/java/com/palantir/tritium/proxy/InvocationEventProxyTest.java +++ b/tritium-lib/src/test/java/com/palantir/tritium/proxy/InvocationEventProxyTest.java @@ -142,8 +142,8 @@ public void testToInvocationDebugString() throws Exception { InvocationEventProxy proxy = createBasicProxy( Collections.>emptyList()); - String debugString = proxy.toInvocationDebugString(this, getToStringMethod(), null); - assertThat(debugString, containsString("x")); + Throwable cause = new RuntimeException("cause"); + proxy.logInvocationWarning("test", this, getToStringMethod(), null, cause); } private InvocationEventProxy createBasicProxy( diff --git a/tritium-metrics/build.gradle b/tritium-metrics/build.gradle index e2da21fd5..6678da585 100644 --- a/tritium-metrics/build.gradle +++ b/tritium-metrics/build.gradle @@ -5,6 +5,7 @@ dependencies { compile project(':tritium-core') compile 'com.google.code.findbugs:annotations' compile 'com.google.guava:guava' + compile 'com.palantir.safe-logging:safe-logging' compile 'io.dropwizard.metrics:metrics-core' compile 'org.mpierce.metrics.reservoir:hdrhistogram-metrics-reservoir' compile 'org.hdrhistogram:HdrHistogram' diff --git a/tritium-metrics/src/main/java/com/palantir/tritium/event/metrics/MetricsInvocationEventHandler.java b/tritium-metrics/src/main/java/com/palantir/tritium/event/metrics/MetricsInvocationEventHandler.java index f13bba6ca..a1c0bbcd4 100644 --- a/tritium-metrics/src/main/java/com/palantir/tritium/event/metrics/MetricsInvocationEventHandler.java +++ b/tritium-metrics/src/main/java/com/palantir/tritium/event/metrics/MetricsInvocationEventHandler.java @@ -36,7 +36,7 @@ */ public final class MetricsInvocationEventHandler extends AbstractInvocationEventHandler { - private static final Logger LOGGER = LoggerFactory.getLogger(MetricsInvocationEventHandler.class); + private static final Logger logger = LoggerFactory.getLogger(MetricsInvocationEventHandler.class); private final MetricRegistry metricRegistry; private final String serviceName; @@ -63,7 +63,7 @@ public InvocationContext preInvocation(Object instance, Method method, Object[] @Override public void onSuccess(@Nullable InvocationContext context, @Nullable Object result) { if (context == null) { - LOGGER.debug("Encountered null metric context likely due to exception in preInvocation"); + logger.debug("Encountered null metric context likely due to exception in preInvocation"); return; } metricRegistry.timer(getBaseMetricName(context)) @@ -74,7 +74,7 @@ public void onSuccess(@Nullable InvocationContext context, @Nullable Object resu public void onFailure(@Nullable InvocationContext context, @Nonnull Throwable cause) { if (context == null) { markGlobalFailure(); - LOGGER.debug("Encountered null metric context likely due to exception in preInvocation: {}", cause, cause); + logger.debug("Encountered null metric context likely due to exception in preInvocation: {}", cause, cause); return; } diff --git a/tritium-metrics/src/main/java/com/palantir/tritium/metrics/MetricRegistries.java b/tritium-metrics/src/main/java/com/palantir/tritium/metrics/MetricRegistries.java index 3f0c26fe0..1699c7236 100644 --- a/tritium-metrics/src/main/java/com/palantir/tritium/metrics/MetricRegistries.java +++ b/tritium-metrics/src/main/java/com/palantir/tritium/metrics/MetricRegistries.java @@ -40,7 +40,7 @@ */ public final class MetricRegistries { - private static final Logger LOGGER = LoggerFactory.getLogger(MetricRegistries.class); + private static final Logger logger = LoggerFactory.getLogger(MetricRegistries.class); /** * An ISO 8601 date format for pre-Java 8 compatibility without Joda dependency. @@ -160,7 +160,7 @@ public static T registerSafe(MetricRegistry registry, String if (existingMetric == null) { return registry.register(name, metric); } else { - LOGGER.warn("Metric already registered at this name." + logger.warn("Metric already registered at this name." + " Name: {}, existing metric: {}", name, existingMetric); Set> existingMetricInterfaces = ImmutableSet.copyOf(existingMetric.getClass().getInterfaces()); diff --git a/tritium-slf4j/build.gradle b/tritium-slf4j/build.gradle index 12d3e0a84..0cd79c70f 100644 --- a/tritium-slf4j/build.gradle +++ b/tritium-slf4j/build.gradle @@ -4,6 +4,7 @@ dependencies { compile project(':tritium-api') compile project(':tritium-core') compile 'com.google.code.findbugs:annotations' + compile 'com.palantir.safe-logging:safe-logging' compile 'org.slf4j:slf4j-api' testCompile project(path: ':tritium-core', configuration: 'testArtifacts') diff --git a/tritium-slf4j/src/main/java/com/palantir/tritium/event/log/LoggingInvocationEventHandler.java b/tritium-slf4j/src/main/java/com/palantir/tritium/event/log/LoggingInvocationEventHandler.java index 86638b4c6..5e342824e 100644 --- a/tritium-slf4j/src/main/java/com/palantir/tritium/event/log/LoggingInvocationEventHandler.java +++ b/tritium-slf4j/src/main/java/com/palantir/tritium/event/log/LoggingInvocationEventHandler.java @@ -20,6 +20,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.palantir.logsafe.SafeArg; import com.palantir.tritium.api.functions.BooleanSupplier; import com.palantir.tritium.api.functions.LongPredicate; import com.palantir.tritium.event.AbstractInvocationEventHandler; @@ -40,7 +41,7 @@ */ public class LoggingInvocationEventHandler extends AbstractInvocationEventHandler { - private static final Logger LOGGER = LoggerFactory.getLogger(LoggingInvocationEventHandler.class); + private static final Logger CLASS_LOGGER = LoggerFactory.getLogger(LoggingInvocationEventHandler.class); private static final List MESSAGE_PATTERNS = generateMessagePatterns(10); public static final LongPredicate LOG_ALL_DURATIONS = LongPredicate.TRUE; @@ -76,7 +77,7 @@ public final InvocationContext preInvocation(Object instance, Method method, Obj @Override public final void onSuccess(@Nullable InvocationContext context, @Nullable Object result) { if (context == null) { - LOGGER.debug("Encountered null metric context likely due to exception in preInvocation"); + CLASS_LOGGER.debug("Encountered null metric context likely due to exception in preInvocation"); return; } @@ -87,7 +88,7 @@ public final void onSuccess(@Nullable InvocationContext context, @Nullable Objec @Override public final void onFailure(@Nullable InvocationContext context, @Nonnull Throwable cause) { if (context == null) { - LOGGER.debug("Encountered null metric context likely due to exception in preInvocation"); + CLASS_LOGGER.debug("Encountered null metric context likely due to exception in preInvocation"); return; } @@ -191,9 +192,9 @@ static String getMessagePattern(Object[] args) { static Object[] getLogParams(Method method, Object[] args, long durationNanos, LoggingLevel level) { Object[] logParams = new Object[3 + args.length]; - logParams[0] = method.getDeclaringClass().getSimpleName(); - logParams[1] = method.getName(); - logParams[logParams.length - 1] = String.format("%.3f", durationNanos / 1000000.0d); + logParams[0] = SafeArg.of("class", method.getDeclaringClass().getSimpleName()); + logParams[1] = SafeArg.of("method", method.getName()); + logParams[logParams.length - 1] = SafeArg.of("milliseconds", String.format("%.3f", durationNanos / 1000000.0d)); Class[] argTypes = method.getParameterTypes(); if (argTypes.length == 0) { @@ -209,7 +210,7 @@ static Object[] getLogParams(Method method, Object[] args, long durationNanos, L } } - logParams[2 + i] = argMessage; + logParams[2 + i] = SafeArg.of("type" + i, argMessage); } return logParams; diff --git a/tritium-tracing/build.gradle b/tritium-tracing/build.gradle index 1502d2f9b..836464278 100644 --- a/tritium-tracing/build.gradle +++ b/tritium-tracing/build.gradle @@ -6,6 +6,7 @@ dependencies { compile 'com.google.code.findbugs:annotations' compile 'com.palantir.remoting1:tracing' + compile 'com.palantir.safe-logging:safe-logging' compile 'org.slf4j:slf4j-api' testCompile project(path: ':tritium-core', configuration: 'testArtifacts') diff --git a/tritium-tracing/src/main/java/com/palantir/tritium/tracing/TracingInvocationEventHandler.java b/tritium-tracing/src/main/java/com/palantir/tritium/tracing/TracingInvocationEventHandler.java index 31100b17c..ff9a80f4b 100644 --- a/tritium-tracing/src/main/java/com/palantir/tritium/tracing/TracingInvocationEventHandler.java +++ b/tritium-tracing/src/main/java/com/palantir/tritium/tracing/TracingInvocationEventHandler.java @@ -30,7 +30,7 @@ public final class TracingInvocationEventHandler extends AbstractInvocationEventHandler { - private static final Logger LOGGER = LoggerFactory.getLogger(TracingInvocationEventHandler.class); + private static final Logger logger = LoggerFactory.getLogger(TracingInvocationEventHandler.class); private final String component; @@ -66,7 +66,7 @@ public void onFailure(@Nullable InvocationContext context, @Nonnull Throwable ca private void debugIfNullContext(@Nullable InvocationContext context) { if (context == null) { - LOGGER.debug("Encountered null metric context likely due to exception in preInvocation"); + logger.debug("Encountered null metric context likely due to exception in preInvocation"); } } diff --git a/versions.props b/versions.props index 9ccd58b75..f9482c1bb 100644 --- a/versions.props +++ b/versions.props @@ -5,6 +5,7 @@ com.google.code.findbugs:jFormatString = 3.0.0 com.google.guava:* = 18.0 com.google.truth:* = 0.30 com.palantir.remoting1:* = 1.0.0 +com.palantir.safe-logging:safe-logging = 0.1.1 io.dropwizard.metrics:metrics-core = 3.1.2 io.zipkin.brave:* = 3.11.1 io.zipkin.java:* = 1.12.1