Skip to content

Commit

Permalink
Renaming LoggingScope to LoggingContextCloseable, since "scopes" are …
Browse files Browse the repository at this point in the history 
…really something else.

RELNOTES=Renaming LoggingScope to LoggingContextCloseable.
PiperOrigin-RevId: 360516485
  • Loading branch information
@hagbard
hagbard authored and Flogger Team committed Mar 2, 2021
1 parent 9348b91 commit 8442b81
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 43 deletions.
8 changes: 4 additions & 4 deletions api/src/main/java/com/google/common/flogger/backend/NoOpContextDataProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
import com.google.common.flogger.FluentLogger;
import com.google.common.flogger.StackSize;
import com.google.common.flogger.context.ContextDataProvider;
import com.google.common.flogger.context.LogLevelMap;
import com.google.common.flogger.context.ScopedLoggingContext;
import com.google.common.flogger.context.ScopedLoggingContext.LoggingScope;
import com.google.common.flogger.context.ScopedLoggingContext.LoggingContextCloseable;
import com.google.common.flogger.context.Tags;
import com.google.common.flogger.context.LogLevelMap;
import java.util.concurrent.atomic.AtomicBoolean;

/**
Expand All @@ -43,7 +43,7 @@ public static final ContextDataProvider getInstance() {
}

private static final class NoOpScopedLoggingContext extends ScopedLoggingContext
implements LoggingScope {
implements LoggingContextCloseable {
// Since the ContextDataProvider class is loaded during Platform initialization we must be very
// careful to avoid any attempt to obtain a logger instance until we can be sure logging config
// is complete.
Expand All @@ -69,7 +69,7 @@ private void logWarningOnceOnly() {
public ScopedLoggingContext.Builder newScope() {
return new ScopedLoggingContext.Builder() {
@Override
public LoggingScope install() {
public LoggingContextCloseable install() {
logWarningOnceOnly();
return NoOpScopedLoggingContext.this;
}
Expand Down
8 changes: 4 additions & 4 deletions api/src/main/java/com/google/common/flogger/backend/system/LoggingContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@

package com.google.common.flogger.backend.system;

import com.google.common.flogger.context.Tags;
import com.google.common.flogger.context.ContextDataProvider;
import com.google.common.flogger.context.LogLevelMap;
import com.google.common.flogger.context.ScopedLoggingContext;
import com.google.common.flogger.context.ScopedLoggingContext.LoggingScope;
import com.google.common.flogger.context.ScopedLoggingContext.LoggingContextCloseable;
import com.google.common.flogger.context.Tags;

/**
* @deprecated Replaced by ContextDataProvider.
Expand All @@ -37,12 +37,12 @@ public ScopedLoggingContext getContextApiSingleton() {
}

private static final class NoOpScopedLoggingContext extends ScopedLoggingContext
implements LoggingScope {
implements LoggingContextCloseable {
@Override
public ScopedLoggingContext.Builder newScope() {
return new ScopedLoggingContext.Builder() {
@Override
public LoggingScope install() {
public LoggingContextCloseable install() {
return NoOpScopedLoggingContext.this;
}
};
Expand Down
72 changes: 44 additions & 28 deletions api/src/main/java/com/google/common/flogger/context/ScopedLoggingContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
public abstract class ScopedLoggingContext {
/** A logging context which must be closed in the reverse order to which it was created. */
// If Flogger is bumped to JDK 1.7, this should be switched to AutoCloseable.
public interface LoggingScope extends Closeable {
public interface LoggingContextCloseable extends Closeable {
// Overridden to remove the throws clause allowing simple try-with-resources use.
@Override
public void close();
Expand Down Expand Up @@ -154,7 +154,7 @@ public final Runnable wrap(final Runnable r) {
@SuppressWarnings("MustBeClosedChecker")
public void run() {
// JDK 1.6 does not have "try-with-resources"
LoggingScope context = install();
LoggingContextCloseable context = install();
boolean hasError = true;
try {
r.run();
Expand All @@ -180,7 +180,7 @@ public final <R> Callable<R> wrap(final Callable<R> c) {
@Override
@SuppressWarnings("MustBeClosedChecker")
public R call() throws Exception {
LoggingScope context = install();
LoggingContextCloseable context = install();
boolean hasError = true;
try {
R result = c.call();
Expand All @@ -198,20 +198,34 @@ public final void run(Runnable r) {
wrap(r).run();
}

/** Calls a callable directly within a new context installed from this builder. */
/** Calls a {@link Callable} directly within a new context installed from this builder. */
public final <R> R call(Callable<R> c) throws Exception {
return wrap(c).call();
}

/**
* Calls a {@link Callable} directly within a new context installed from this builder, wrapping
* any checked exceptions with a {@link RuntimeException}.
*/
public final <R> R callUnchecked(Callable<R> c) {
try {
return call(c);
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
throw new RuntimeException("checked exception caught during context call", e);
}
}

/**
* Installs a new context based on the state of the builder. The caller is <em>required</em> to
* invoke {@link LoggingScope#close() close()} on the returned instances in the reverse order to
* which they were obtained. For JDK 1.7 and above, this is best achieved by using a
* try-with-resources construction in the calling code.
* invoke {@link LoggingContextCloseable#close() close()} on the returned instances in the
* reverse order to which they were obtained. For JDK 1.7 and above, this is best achieved by
* using a try-with-resources construction in the calling code.
*
* <pre>{@code
* ScopedLoggingContext ctx = ScopedLoggingContext.getInstance();
* try (LoggingScope scope = ctx.newScope().withTags(Tags.of("my_tag", someValue).install()) {
* try (LoggingContextCloseable ctx = ScopedLoggingContext.getInstance()
* .newScope().withTags(Tags.of("my_tag", someValue).install()) {
* // Any logging by code called from within this context will contain the additional metadata.
* logger.atInfo().log("Log message should contain tag value...");
* }
Expand All @@ -225,14 +239,14 @@ public final <R> R call(Callable<R> c) throws Exception {
* <p>An implementation of scoped contexts must preserve any existing metadata when a context is
* opened, and restore the previous state when it terminates.
*
* <p>Note that the returned {@link LoggingScope} is not required to enforce the correct closure
* of nested contexts, and while it is permitted to throw a {@link
* <p>Note that the returned {@link LoggingContextCloseable} is not required to enforce the
* correct closure of nested contexts, and while it is permitted to throw a {@link
* InvalidLoggingScopeStateException} in the face of mismatched or invalid usage, it is not
* required.
*/
@MustBeClosed
@CheckReturnValue
public abstract LoggingScope install();
public abstract LoggingContextCloseable install();

/**
* Returns the configured tags, or null. This method may do work and results should be cached by
Expand Down Expand Up @@ -291,13 +305,13 @@ protected ScopedLoggingContext() {}

/**
* Installs a new context to which additional logging metadata can be attached. The caller is
* <em>required</em> to invoke {@link LoggingScope#close() close()} on the returned instances in
* the reverse order to which they were obtained. For JDK 1.7 and above, this is best achieved by
* using a try-with-resources construction in the calling code.
* <em>required</em> to invoke {@link LoggingContextCloseable#close() close()} on the returned
* instances in the reverse order to which they were obtained. For JDK 1.7 and above, this is best
* achieved by using a try-with-resources construction in the calling code.
*
* <pre>{@code
* ScopedLoggingContext ctx = ScopedLoggingContext.getInstance();
* try (LoggingScope context = ctx.withNewScope()) {
* try (LoggingContextCloseable context = ctx.withNewScope()) {
* // Now add metadata to the installed context (returns false if not supported).
* ctx.addTags(Tags.of("my_tag", someValue));
*
Expand All @@ -317,16 +331,16 @@ protected ScopedLoggingContext() {}
* <pre>{@code
* ScopedLoggingContext ctx = ScopedLoggingContext.getInstance();
* logger.atInfo().log("Some log statement with existing tags and behaviour...");
* try (LoggingScope context = ctx.withNewScope()) {
* try (LoggingContextCloseable context = ctx.withNewScope()) {
* logger.atInfo().log("This log statement is the same as the first...");
* ctx.addTags(Tags.of("my_tag", someValue));
* logger.atInfo().log("This log statement has the new tag present...");
* }
* logger.atInfo().log("This log statement is the same as the first...");
* }</pre>
*
* <p>Note that the returned {@link LoggingScope} is not required to enforce the correct closure
* of nested contexts, and while it is permitted to throw a {@link
* <p>Note that the returned {@link LoggingContextCloseable} is not required to enforce the
* correct closure of nested contexts, and while it is permitted to throw a {@link
* InvalidLoggingScopeStateException} in the face of mismatched or invalid usage, it is not
* required.
*
Expand All @@ -339,7 +353,7 @@ protected ScopedLoggingContext() {}
@Deprecated
@MustBeClosed
@CheckReturnValue
public final LoggingScope withNewScope() {
public final LoggingContextCloseable withNewScope() {
return newScope().install();
}

Expand Down Expand Up @@ -445,8 +459,10 @@ public boolean applyLogLevelMap(LogLevelMap logLevelMap) {
return false;
}

private static void closeAndMaybePropagateError(LoggingScope context, boolean callerHasError) {
// Because LoggingScope is not just a "Closeable" there's no risk of it throwing any checked
private static void closeAndMaybePropagateError(
LoggingContextCloseable context, boolean callerHasError) {
// Because LoggingContextCloseable is not just a "Closeable" there's no risk of it throwing any
// checked
// exceptions. Inparticular, when this is switched to use AutoCloseable, there's no risk of
// having to deal with InterruptedException. That's why having an extended interface is always
// better than using [Auto]Closeable directly.
Expand All @@ -456,9 +472,9 @@ private static void closeAndMaybePropagateError(LoggingScope context, boolean ca
// This method is always called from a finally block which may be about to rethrow a user
// exception, so ignore any errors during close() if that's the case.
if (!callerHasError) {
throw (e instanceof InvalidLoggingScopeStateException)
? ((InvalidLoggingScopeStateException) e)
: new InvalidLoggingScopeStateException("invalid logging context state", e);
throw (e instanceof InvalidLoggingContextStateException)
? ((InvalidLoggingContextStateException) e)
: new InvalidLoggingContextStateException("invalid logging context state", e);
}
}
}
Expand All @@ -468,12 +484,12 @@ private static void closeAndMaybePropagateError(LoggingScope context, boolean ca
* at which this exception is thrown may not itself be the point where the mishandling occurred,
* but simply where it was first detected.
*/
public static final class InvalidLoggingScopeStateException extends IllegalStateException {
public InvalidLoggingScopeStateException(String message, Throwable cause) {
public static final class InvalidLoggingContextStateException extends IllegalStateException {
public InvalidLoggingContextStateException(String message, Throwable cause) {
super(message, cause);
}

public InvalidLoggingScopeStateException(String message) {
public InvalidLoggingContextStateException(String message) {
super(message);
}
}
Expand Down
10 changes: 5 additions & 5 deletions api/src/test/java/com/google/common/flogger/context/ScopedLoggingContextTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;

import com.google.common.flogger.context.ScopedLoggingContext.LoggingScope;
import com.google.common.flogger.context.ScopedLoggingContext.InvalidLoggingScopeStateException;
import com.google.common.flogger.context.ScopedLoggingContext.InvalidLoggingContextStateException;
import com.google.common.flogger.context.ScopedLoggingContext.LoggingContextCloseable;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand All @@ -37,7 +37,7 @@ public class ScopedLoggingContextTest {
public ScopedLoggingContext.Builder newScope() {
return new ScopedLoggingContext.Builder() {
@Override
public LoggingScope install() {
public LoggingContextCloseable install() {
return () -> {
throw new IllegalArgumentException("BAD CONTEXT");
};
Expand All @@ -58,8 +58,8 @@ public boolean addTags(Tags tags) {

@Test
public void testErrorHandlingWithoutUserError() {
InvalidLoggingScopeStateException e =
assertThrows(InvalidLoggingScopeStateException.class, () -> ERROR_CONTEXT.run(() -> {}));
InvalidLoggingContextStateException e =
assertThrows(InvalidLoggingContextStateException.class, () -> ERROR_CONTEXT.run(() -> {}));
assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("BAD CONTEXT");
}
Expand Down
4 changes: 2 additions & 2 deletions grpc/src/main/java/com/google/common/flogger/grpc/GrpcScopedLoggingContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ static ScopedLoggingContext getGrpcConfigInstance() {
public ScopedLoggingContext.Builder newScope() {
return new ScopedLoggingContext.Builder() {
@Override
public LoggingScope install() {
public LoggingContextCloseable install() {
GrpcContextData newContextData =
GrpcContextData.create(GrpcContextDataProvider.currentContext());
newContextData.addTags(getTags());
Expand All @@ -57,7 +57,7 @@ public LoggingScope install() {
};
}

private static LoggingScope installContextData(GrpcContextData newContextData) {
private static LoggingContextCloseable installContextData(GrpcContextData newContextData) {
// Capture these variables outside the lambda.
Context context =
Context.current().withValue(GrpcContextDataProvider.getContextKey(), newContextData);
Expand Down

0 comments on commit 8442b81

Please sign in to comment.