Skip to content

Commit 6d44932

Browse files
xgnehznick-someone
authored andcommitted
Avoid printing out an injected android log tag found in the metadata. The log tag is already handled separately, so it would be redundant to print it again in the message as part of the "[CONTEXT]" suffix.
The log tag is in the metadata when we do injection of the log tag ([] ). RELNOTES=n/a ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=303161353
1 parent 0a65980 commit 6d44932

File tree

1 file changed

+56
-12
lines changed

1 file changed

+56
-12
lines changed

api/src/main/java/com/google/common/flogger/backend/SimpleMessageFormatter.java

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import static com.google.common.flogger.util.Checks.checkNotNull;
2323

2424
import com.google.common.flogger.LogContext;
25-
import com.google.common.flogger.LogContext.Key;
2625
import com.google.common.flogger.LogSite;
2726
import com.google.common.flogger.MetadataKey;
2827
import com.google.common.flogger.parameter.DateTimeFormat;
@@ -74,6 +73,11 @@ enum Option {
7473
WITH_LOG_SITE
7574
}
7675

76+
/** A predicate to determine which metadata entries should be formatted. */
77+
public interface MetadataPredicate {
78+
boolean shouldFormat(MetadataKey<?> key);
79+
}
80+
7781
// Literal string to be inlined whenever a placeholder references a non-existent argument.
7882
private static final String MISSING_ARGUMENT_MESSAGE = "[ERROR: MISSING LOG ARGUMENT]";
7983

@@ -84,6 +88,17 @@ enum Option {
8488
// removes the capability of optimising certain formatting operations.
8589
private static final Locale FORMAT_LOCALE = Locale.ROOT;
8690

91+
// Default metadata keys to add to formatted strings. (No lambdas here for compatibility.)
92+
// When the Flogger core library supports JDK 8, this can be converted to a lambda or Predicate.
93+
@SuppressWarnings("UnnecessaryAnonymousClass")
94+
private static final MetadataPredicate FORMAT_ALL_METADATA =
95+
new MetadataPredicate() {
96+
@Override
97+
public boolean shouldFormat(MetadataKey<?> key) {
98+
return true;
99+
}
100+
};
101+
87102
/**
88103
* Formats the log message and any metadata for the given {@link LogData}, calling the supplied
89104
* receiver object with the results.
@@ -97,20 +112,39 @@ public static void format(LogData logData, SimpleLogHandler receiver) {
97112
* receiver object with the results with a given option.
98113
*/
99114
static void format(LogData logData, SimpleLogHandler receiver, Option option) {
115+
format(logData, receiver, option, FORMAT_ALL_METADATA);
116+
}
117+
118+
/**
119+
* Formats the log message and any metadata for the given {@link LogData}, calling the supplied
120+
* receiver object with the results with a given option and metadata keys filter.
121+
*/
122+
static void format(
123+
LogData logData,
124+
SimpleLogHandler receiver,
125+
Option option,
126+
MetadataPredicate metadataPredicate) {
100127
Metadata metadata = logData.getMetadata();
101128
Throwable thrown = metadata.findValue(LogContext.Key.LOG_CAUSE);
102-
// Either no metadata, or exactly one "cause" means we don't need to do additional formatting.
103-
// This is pretty common and will save some work and object allocations.
104-
boolean hasOnlyKnownMetadata = metadata.size() == 0 || (metadata.size() == 1 && thrown != null);
129+
// Either no metadata, or only "cause" and ignored metadata means we don't need to do additional
130+
// formatting. This is pretty common and will save some work and object allocations.
131+
boolean hasOnlyKnownMetadata = true;
132+
for (int n = 0; n < metadata.size(); n++) {
133+
MetadataKey<?> key = metadata.getKey(n);
134+
if (shouldFormat(key, metadataPredicate)) {
135+
hasOnlyKnownMetadata = false;
136+
break;
137+
}
138+
}
105139

106140
TemplateContext ctx = logData.getTemplateContext();
107141
String message;
108142
if (ctx == null) {
109-
message = formatLiteralMessage(logData, option, hasOnlyKnownMetadata);
143+
message = formatLiteralMessage(logData, option, hasOnlyKnownMetadata, metadataPredicate);
110144
} else {
111145
StringBuilder buffer = formatMessage(logData, option);
112146
if (!hasOnlyKnownMetadata) {
113-
appendContext(buffer, metadata);
147+
appendContext(buffer, metadata, metadataPredicate);
114148
}
115149
message = buffer.toString();
116150
}
@@ -202,15 +236,16 @@ private static StringBuilder formatMessage(LogData logData, Option option) {
202236
return out;
203237
}
204238

205-
private static void appendContext(StringBuilder out, Metadata metadata) {
239+
private static void appendContext(
240+
StringBuilder out, Metadata metadata, MetadataPredicate metadataPredicate) {
206241
KeyValueFormatter kvf = new KeyValueFormatter("[CONTEXT ", " ]", out);
207242
Tags tags = null;
208243
for (int n = 0; n < metadata.size(); n++) {
209244
MetadataKey<?> key = metadata.getKey(n);
210-
if (key.equals(Key.LOG_CAUSE)) {
245+
if (!shouldFormat(key, metadataPredicate)) {
211246
continue;
212-
} else if (key.equals(Key.TAGS)) {
213-
tags = Key.TAGS.cast(metadata.getValue(n));
247+
} else if (key.equals(LogContext.Key.TAGS)) {
248+
tags = LogContext.Key.TAGS.cast(metadata.getValue(n));
214249
continue;
215250
}
216251
key.emit(metadata.getValue(n), kvf);
@@ -221,6 +256,12 @@ private static void appendContext(StringBuilder out, Metadata metadata) {
221256
kvf.done();
222257
}
223258

259+
private static boolean shouldFormat(MetadataKey<?> key, MetadataPredicate metadataPredicate) {
260+
// The cause is special and is never formatted like other metadata (it's also the most common,
261+
// so checking for it first is good).
262+
return !key.equals(LogContext.Key.LOG_CAUSE) && metadataPredicate.shouldFormat(key);
263+
}
264+
224265
// Input argument array reference (not copied).
225266
private final Object[] args;
226267
// Buffer into which the message is formatted.
@@ -405,7 +446,10 @@ private static void appendInvalid(StringBuilder out, Object value, String format
405446
}
406447

407448
private static String formatLiteralMessage(
408-
LogData logData, Option option, boolean hasOnlyKnownMetadata) {
449+
LogData logData,
450+
Option option,
451+
boolean hasOnlyKnownMetadata,
452+
MetadataPredicate metadataPredicate) {
409453
// If a literal message (no arguments) is logged and no metadata exists, just use the string.
410454
// Having no format arguments is fairly common and this avoids allocating StringBuilders and
411455
// formatter instances in a lot of situations.
@@ -421,7 +465,7 @@ private static String formatLiteralMessage(
421465
if (!hasOnlyKnownMetadata) {
422466
// If unknown metadata exists we have to append it to the message (this is not that common
423467
// because a Throwable "cause" is already handled separately).
424-
appendContext(builder, logData.getMetadata());
468+
appendContext(builder, logData.getMetadata(), metadataPredicate);
425469
}
426470
return builder.toString();
427471
}

0 commit comments

Comments
 (0)