diff --git a/api/src/main/java/com/google/common/flogger/LogContext.java b/api/src/main/java/com/google/common/flogger/LogContext.java
index 4864c4ed..38887d21 100644
--- a/api/src/main/java/com/google/common/flogger/LogContext.java
+++ b/api/src/main/java/com/google/common/flogger/LogContext.java
@@ -517,7 +517,7 @@ protected boolean postProcess(@NullableDecl LogSiteKey logSiteKey) {
* of arguments.
*/
private boolean shouldLog() {
- // The log site may have already been injected via "withInjectedLogSite()".
+ // The log site may have already been injected via "withInjectedLogSite()" or similar.
if (logSite == null) {
// From the point at which we call inferLogSite() we can skip 1 additional method (the
// shouldLog() method itself) when looking up the stack to find the log() method.
@@ -571,9 +571,11 @@ private void logImpl(String message, Object... args) {
@Override
public final API withInjectedLogSite(LogSite logSite) {
// First call wins (since auto-injection will typically target the log() method at the end of
- // the chain and might not check for previous explicit injection).
- if (this.logSite == null) {
- this.logSite = checkNotNull(logSite, "log site");
+ // the chain and might not check for previous explicit injection). In particular it MUST be
+ // allowed for a caller to specify the "INVALID" log site, and have that set the field here to
+ // disable log site lookup at this log statement (though passing "null" is a no-op).
+ if (this.logSite == null && logSite != null) {
+ this.logSite = logSite;
}
return api();
}
diff --git a/api/src/main/java/com/google/common/flogger/LogSites.java b/api/src/main/java/com/google/common/flogger/LogSites.java
index 5feaf3d9..3c367821 100644
--- a/api/src/main/java/com/google/common/flogger/LogSites.java
+++ b/api/src/main/java/com/google/common/flogger/LogSites.java
@@ -26,16 +26,83 @@
*
*/
public final class LogSites {
+ /**
+ * Returns a {@code LogSite} for the caller of the specified class. This can be used in
+ * conjunction with the {@link LoggingApi#withInjectedLogSite(LogSite)} method to implement
+ * logging helper methods. In some platforms, log site determination may be unsupported, and in
+ * those cases this method will always return the {@link LogSite#INVALID} instance.
+ *
+ * For example (in {@code MyLoggingHelper}):
+ *
{@code
+ * public static void logAndSomethingElse(String message, Object... args) {
+ * logger.atInfo()
+ * .withInjectedLogSite(callerOf(MyLoggingHelper.class))
+ * .logVarargs(message, args);
+ * }
+ * }
+ *
+ * This method should be used for the simple cases where the class in which the logging occurs is
+ * a public logging API. If the log statement is in a different class (not the public logging API)
+ * and the {@code LogSite} instance needs to be passed through several layers, consider using
+ * {@link #logSite()} instead to avoid too much "magic" in your code.
+ *
+ * You should also seek to ensure that any API used with this method "looks like a logging API".
+ * It's no good if a log entry contains a class and method name which doesn't correspond to
+ * anything the user can relate to. In particular, the API should probably always accept the log
+ * message or at least some of its parameters, and should always have methods with "log" in their
+ * names to make the connection clear.
+ *
+ * It is very important to note that this method can be very slow, since determining the log site
+ * can involve stack trace analysis. It is only recommended that it is used for cases where
+ * logging is expected to occur (e.g. {@code INFO} level or above). Implementing a helper method
+ * for {@code FINE} logging is usually unnecessary (it doesn't normally need to follow any
+ * specific "best practice" behavior).
+ *
+ * Note that even when log site determination is supported, it is not defined as to whether two
+ * invocations of this method on the same line of code will produce the same instance, equivalent
+ * instances or distinct instance. Thus you should never invoke this method twice in a single
+ * statement (and you should never need to).
+ *
+ * Note that this method call may be replaced in compiled applications via bytecode manipulation
+ * or other mechanisms to improve performance.
+ *
+ * @param loggingApi the logging API to be identified as the source of log statements (this must
+ * appear somewhere on the stack above the point at which this method is called).
+ * @return the log site of the caller of the specified logging API, or {@link LogSite#INVALID} if
+ * the logging API was not found.
+ */
+ public static LogSite callerOf(Class loggingApi) {
+ // Can't skip anything here since someone could pass in LogSite.class.
+ return Platform.getCallerFinder().findLogSite(loggingApi, 0);
+ }
+
/**
* Returns a {@code LogSite} for the current line of code. This can be used in conjunction with
* the {@link LoggingApi#withInjectedLogSite(LogSite)} method to implement logging helper
* methods. In some platforms, log site determination may be unsupported, and in those cases this
* method will always return the {@link LogSite#INVALID} instance.
*
+ * For example (in {@code MyLoggingHelper}):
+ *
{@code
+ * public static void logAndSomethingElse(LogSite logSite, String message, Object... args) {
+ * logger.atInfo()
+ * .withInjectedLogSite(logSite)
+ * .logVarargs(message, args);
+ * }
+ * }
+ * where callers would do:
+ * {@code
+ * MyLoggingHelper.logAndSomethingElse(logSite(), "message...");
+ * }
+ *
+ * Because this method adds an additional parameter and exposes a Flogger specific type to the
+ * calling code, you should consider using {@link #callerOf(Class)} for simple logging
+ * utilities.
+ *
* It is very important to note that this method can be very slow, since determining the log site
* can involve stack trace analysis. It is only recommended that it is used for cases where
- * logging is expected to occur (e.g. {@code WARNING} level or above). Implementing a helper
- * method for {@code FINE} logging is usually unnecessary (it doesn't normally need to follow any
+ * logging is expected to occur (e.g. {@code INFO} level or above). Implementing a helper method
+ * for {@code FINE} logging is usually unnecessary (it doesn't normally need to follow any
* specific "best practice" behavior).
*
* Note that even when log site determination is supported, it is not defined as to whether two
@@ -45,8 +112,11 @@ public final class LogSites {
*
* Note that this method call may be replaced in compiled applications via bytecode manipulation
* or other mechanisms to improve performance.
+ *
+ * @return the log site of the caller of this method.
*/
public static LogSite logSite() {
+ // Don't call "callerOf()" to avoid making another stack entry.
return Platform.getCallerFinder().findLogSite(LogSites.class, 0);
}
diff --git a/api/src/main/java/com/google/common/flogger/LoggingApi.java b/api/src/main/java/com/google/common/flogger/LoggingApi.java
index e16a0636..f1ae7eec 100644
--- a/api/src/main/java/com/google/common/flogger/LoggingApi.java
+++ b/api/src/main/java/com/google/common/flogger/LoggingApi.java
@@ -627,7 +627,6 @@ protected final API noOp() {
@Override
public API withInjectedLogSite(LogSite logSite) {
- checkNotNull(logSite, "log site");
return noOp();
}
diff --git a/api/src/test/java/com/google/common/flogger/LogContextTest.java b/api/src/test/java/com/google/common/flogger/LogContextTest.java
index 2aedffaa..2264fa76 100644
--- a/api/src/test/java/com/google/common/flogger/LogContextTest.java
+++ b/api/src/test/java/com/google/common/flogger/LogContextTest.java
@@ -544,7 +544,6 @@ public void testStackTraceFormatting() {
// Print the stack trace via the expected method (ie, printStackTrace()).
Throwable cause = backend.getLogged(0).getMetadata().findValue(Key.LOG_CAUSE);
assertThat(cause).hasMessageThat().isEqualTo("SMALL");
- ;
StringWriter out = new StringWriter();
cause.printStackTrace(new PrintWriter(out));
Iterable actualStackLines = Splitter.on('\n').trimResults().split(out.toString());
@@ -598,9 +597,11 @@ private static StackTraceElement getCallerInfoFollowingLine() {
public void testExplicitLogSiteInjection() {
FakeLoggerBackend backend = new FakeLoggerBackend();
FluentLogger logger = new FluentLogger(backend);
+ // Tests it's the log site instance that controls rate limiting, even over different calls.
+ // We don't expect this to ever happen in real code though.
for (int i = 0; i <= 6; i++) {
- logHelper(logger, logSite(), 2, "Foo: " + i);
- logHelper(logger, logSite(), 3, "Bar: " + i);
+ logHelper(logger, logSite(), 2, "Foo: " + i); // Log every 2nd (0, 2, 4, 6)
+ logHelper(logger, logSite(), 3, "Bar: " + i); // Log every 3rd (0, 3, 6)
}
// Expect: Foo -> 0, 2, 4, 6 and Bar -> 0, 3, 6 (but not in that order)
assertThat(backend.getLoggedCount()).isEqualTo(7);
@@ -617,4 +618,21 @@ public void testExplicitLogSiteInjection() {
private static void logHelper(FluentLogger logger, LogSite logSite, int n, String message) {
logger.atInfo().withInjectedLogSite(logSite).every(n).log(message);
}
+
+ // It's important that injecting an INVALID log site acts as a override to suppress log site
+ // calculation rather than being a no-op.
+ @Test
+ public void testExplicitLogSiteSuppression() {
+ FakeLoggerBackend backend = new FakeLoggerBackend();
+ FluentLogger logger = new FluentLogger(backend);
+
+ logger.atInfo().withInjectedLogSite(LogSite.INVALID).log("No log site here");
+ logger.atInfo().withInjectedLogSite(null).log("No-op injection");
+
+ assertThat(backend.getLoggedCount()).isEqualTo(2);
+ backend.assertLogged(0).logSite().isEqualTo(LogSite.INVALID);
+
+ backend.assertLogged(1).logSite().isNotNull();
+ backend.assertLogged(1).logSite().isNotEqualTo(LogSite.INVALID);
+ }
}
diff --git a/api/src/test/java/com/google/common/flogger/LogSitesTest.java b/api/src/test/java/com/google/common/flogger/LogSitesTest.java
new file mode 100644
index 00000000..08f13537
--- /dev/null
+++ b/api/src/test/java/com/google/common/flogger/LogSitesTest.java
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2020 The Flogger Authors.
+ *
+ * Licensed 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 com.google.common.flogger;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public final class LogSitesTest {
+ @Test
+ public void testLogSite() {
+ assertThat(LogSites.logSite().getMethodName()).isEqualTo("testLogSite");
+ }
+
+ @Test
+ public void testCallerOf() {
+ assertThat(MyLogUtil.getCallerLogSite().getMethodName()).isEqualTo("testCallerOf");
+ assertThat(MyLogUtil.getCallerLogSiteWrapped().getMethodName()).isEqualTo("testCallerOf");
+ }
+
+ @Test
+ public void testCallerOf_notFound() {
+ assertThat(LogSites.callerOf(String.class)).isEqualTo(LogSite.INVALID);
+ }
+
+ private static class MyLogUtil {
+ static LogSite getCallerLogSite() {
+ return LogSites.callerOf(MyLogUtil.class);
+ }
+
+ static LogSite getCallerLogSiteWrapped() {
+ return getCallerLogSite();
+ }
+ }
+}
diff --git a/api/src/test/java/com/google/common/flogger/testing/LogDataSubject.java b/api/src/test/java/com/google/common/flogger/testing/LogDataSubject.java
index a27e244a..7f59b157 100644
--- a/api/src/test/java/com/google/common/flogger/testing/LogDataSubject.java
+++ b/api/src/test/java/com/google/common/flogger/testing/LogDataSubject.java
@@ -90,4 +90,11 @@ public void wasForced() {
failWithActual(simpleFact("expected to be forced"));
}
}
+
+ /**
+ * Asserts about the log site of the log record.
+ */
+ public Subject logSite() {
+ return check("getLogSite()").that(actual.getLogSite());
+ }
}