From 673b79552111125202271e0d662e8519087ef725 Mon Sep 17 00:00:00 2001 From: minherz Date: Sun, 2 Jan 2022 16:42:06 +0000 Subject: [PATCH 1/8] chore: refactoring improve javadoc comments for methods. remove warnings. make methods with no modifier to be private. --- .../logging/logback/LoggingAppender.java | 106 ++++++++++-------- 1 file changed, 59 insertions(+), 47 deletions(-) diff --git a/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java b/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java index e1fb253aa..4556f4988 100644 --- a/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java +++ b/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java @@ -38,6 +38,7 @@ import com.google.common.collect.ImmutableList; import java.io.FileInputStream; import java.io.IOException; +import java.time.Instant; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -106,9 +107,9 @@ public class LoggingAppender extends UnsynchronizedAppenderBase { private final Set loggingEventEnhancerClassNames = new HashSet<>(); /** - * Batched logging requests get immediately flushed for logs at or above this level. + * Sets a threshold for log severity level to flush all log entries that were batched so far. * - *

Defaults to Error if not set. + *

Defaults to Error. * * @param flushLevel Logback log level */ @@ -117,44 +118,48 @@ public void setFlushLevel(Level flushLevel) { } /** - * Sets the log filename. + * Sets the LOG_ID part of the https://cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry#FIELDS.log_name + * for which the logs are ingested. * - * @param log filename + * @param log LOG_ID part of the name */ public void setLog(String log) { this.log = log; } /** - * Sets the name of the monitored resource (Optional). + * Sets the name of the monitored resource (Optional). If not define the appender will try to + * identify the resource type automatically. Currently support resource types include "gae_app", + * "gce_instance", "k8s_container", "cloud_run_revision" and "cloud_function". If the appender + * fails to identify the resource type it will be set to "global". * - *

Must be a supported resource type. - * gae_app, gce_instance and container are auto-detected. + *

Must be a one of the supported resource types. * - *

Defaults to "global" - * - * @param resourceType name of the monitored resource + * @param resourceType the name of the monitored resource. */ public void setResourceType(String resourceType) { this.resourceType = resourceType; } /** - * Sets the credentials file to use to create the {@link LoggingOptions}. The credentials returned - * by {@link GoogleCredentials#getApplicationDefault()} will be used if no custom credentials file - * has been set. + * Sets the path to the https://cloud.google.com/iam/docs/creating-managing-service-account-keys. If not set + * the appender will use {@link GoogleCredentials#getApplicationDefault()} to authenticate. * - * @param credentialsFile The credentials file to use. + * @param credentialsFile the path to the credentials file. */ public void setCredentialsFile(String credentialsFile) { this.credentialsFile = credentialsFile; } /** - * Define synchronization mode for writing log entries. + * Sets the log ingestion mode. It can be one of the {@link Synchronicity} values. + * + *

Default to `Synchronicity.ASYNC`. * - * @param flag to set {@code Synchronicity} value. + * @param flag the new ingestion mode. */ public void setWriteSynchronicity(Synchronicity flag) { this.writeSyncFlag = flag; @@ -169,57 +174,61 @@ public void addLoggingEventEnhancer(String enhancerClassName) { this.loggingEventEnhancerClassNames.add(enhancerClassName); } - Level getFlushLevel() { - return (flushLevel != null) ? flushLevel : Level.ERROR; + /** + * Returns the current value of the ingestion mode. + * + *

The method is deprecated. Use appender configuration to setup the ingestion + * + * @return a {@link Synchronicity} value of the ingestion module. + */ + @Deprecated + public Synchronicity getWriteSynchronicity() { + return (this.writeSyncFlag != null) ? this.writeSyncFlag : Synchronicity.ASYNC; } - String getLogName() { - return (log != null) ? log : "java.log"; + private Level getFlushLevel() { + return (flushLevel != null) ? flushLevel : Level.ERROR; } - public Synchronicity getWriteSynchronicity() { - return (this.writeSyncFlag != null) ? this.writeSyncFlag : Synchronicity.ASYNC; + private String getLogName() { + return (log != null) ? log : "java.log"; } - MonitoredResource getMonitoredResource(String projectId) { + private MonitoredResource getMonitoredResource(String projectId) { return MonitoredResourceUtil.getResource(projectId, resourceType); } - List getLoggingEnhancers() { - return getEnhancers(enhancerClassNames); + private List getLoggingEnhancers() { + return getEnhancers(enhancerClassNames, LoggingEnhancer.class); } - List getLoggingEventEnhancers() { + private List getLoggingEventEnhancers() { if (loggingEventEnhancerClassNames.isEmpty()) { return DEFAULT_LOGGING_EVENT_ENHANCERS; } else { - return getEnhancers(loggingEventEnhancerClassNames); + return getEnhancers(loggingEventEnhancerClassNames, LoggingEventEnhancer.class); } } - List getEnhancers(Set classNames) { - List loggingEnhancers = new ArrayList<>(); + private List getEnhancers(Set classNames, Class classOfT) { + List enhancers = new ArrayList<>(); if (classNames != null) { - for (String enhancerClassName : classNames) { - if (enhancerClassName != null) { - T enhancer = getEnhancer(enhancerClassName); - if (enhancer != null) { - loggingEnhancers.add(enhancer); + for (String className : classNames) { + if (className != null) { + try { + T enhancer = + Loader.loadClass(className.trim()) + .asSubclass(classOfT) + .getDeclaredConstructor() + .newInstance(); + enhancers.add(enhancer); + } catch (Exception ex) { + // invalid className: ignore } } } } - return loggingEnhancers; - } - - private T getEnhancer(String enhancerClassName) { - try { - Class clz = (Class) Loader.loadClass(enhancerClassName.trim()); - return clz.getDeclaredConstructor().newInstance(); - } catch (Exception ex) { - // If we cannot create the enhancer we fallback to null - } - return null; + return enhancers; } /** Initialize and configure the cloud logging service. */ @@ -228,6 +237,9 @@ public synchronized void start() { if (isStarted()) { return; } + + // create new Logging instance + // MonitoredResource resource = getMonitoredResource(getProjectId()); defaultWriteOptions = new WriteOption[] {WriteOption.logName(getLogName()), WriteOption.resource(resource)}; @@ -328,7 +340,7 @@ private LogEntry logEntryFor(ILoggingEvent e) { } LogEntry.Builder builder = LogEntry.newBuilder(Payload.JsonPayload.of(jsonContent)) - .setTimestamp(e.getTimeStamp()) + .setTimestamp(Instant.ofEpochMilli(e.getTimeStamp())) .setSeverity(severity); builder .addLabel(LEVEL_NAME_KEY, level.toString()) From c21514cdb6fd32edb12f797ee709e5150c369ff1 Mon Sep 17 00:00:00 2001 From: minherz Date: Tue, 11 Jan 2022 10:31:22 +0000 Subject: [PATCH 2/8] chore: fix warnings --- .../cloud/logging/logback/LoggingAppenderTest.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java index ab6cd8346..6cb2a01b2 100644 --- a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java @@ -37,6 +37,7 @@ import com.google.cloud.logging.Payload.JsonPayload; import com.google.cloud.logging.Severity; import com.google.common.collect.ImmutableMap; +import java.time.Instant; import java.util.HashMap; import java.util.Map; import org.easymock.Capture; @@ -88,7 +89,7 @@ public void testFlushLevelConfigUpdatesLoggingFlushSeverity() { JsonPayload payload = JsonPayload.of(jsonContent); LogEntry logEntry = LogEntry.newBuilder(payload) - .setTimestamp(100000L) + .setTimestamp(Instant.ofEpochMilli(100000L)) .setSeverity(Severity.WARNING) .setLabels( new ImmutableMap.Builder() @@ -130,7 +131,7 @@ public void testFilterLogsOnlyLogsAtOrAboveLogLevel() { JsonPayload payload = JsonPayload.of(jsonContent); LogEntry logEntry = LogEntry.newBuilder(payload) - .setTimestamp(100000L) + .setTimestamp(Instant.ofEpochMilli(100000L)) .setSeverity(Severity.ERROR) .setLabels( new ImmutableMap.Builder() @@ -168,7 +169,7 @@ public void testEnhancersAddCorrectLabelsToLogEntries() { JsonPayload payload = JsonPayload.of(jsonContent); LogEntry logEntry = LogEntry.newBuilder(payload) - .setTimestamp(100000L) + .setTimestamp(Instant.ofEpochMilli(100000L)) .setSeverity(Severity.WARNING) .setLabels( new ImmutableMap.Builder() @@ -200,7 +201,8 @@ public void testDefaultWriteOptionsHasExpectedDefaults() { logging.setFlushSeverity(Severity.ERROR); Capture logNameArg = Capture.newInstance(); Capture resourceArg = Capture.newInstance(); - logging.write((Iterable) anyObject(), capture(logNameArg), capture(resourceArg)); + logging.write( + EasyMock.>anyObject(), capture(logNameArg), capture(resourceArg)); expectLastCall().once(); replay(logging); loggingAppender.start(); @@ -220,7 +222,7 @@ public void testMdcValuesAreConvertedToLabels() { JsonPayload payload = JsonPayload.of(jsonContent); LogEntry logEntry = LogEntry.newBuilder(payload) - .setTimestamp(100000L) + .setTimestamp(Instant.ofEpochMilli(100000L)) .setSeverity(Severity.INFO) .setLabels( new ImmutableMap.Builder() From 6b7dd15c577f75fb3ff2ead1c5a48f3286f537af Mon Sep 17 00:00:00 2001 From: minherz Date: Wed, 12 Jan 2022 09:35:20 +0000 Subject: [PATCH 3/8] feat: add metadata auto-population and redirect to stdout support add configuration to opt-out metadata auto-population. add configuration to redirect appender output to stdout. add relevant unit tests. refactor unit tests. --- .../logging/logback/LoggingAppender.java | 93 ++++- .../enhancers/AnotherTestLoggingEnhancer.java | 28 -- .../enhancers/TestLoggingEnhancer.java | 28 -- .../logging/logback/LoggingAppenderTest.java | 334 +++++++++++------- 4 files changed, 279 insertions(+), 204 deletions(-) delete mode 100644 src/test/java/com/example/enhancers/AnotherTestLoggingEnhancer.java delete mode 100644 src/test/java/com/example/enhancers/TestLoggingEnhancer.java diff --git a/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java b/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java index 4556f4988..61cbd3dae 100644 --- a/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java +++ b/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java @@ -59,15 +59,21 @@ * <level>INFO</level> * </filter> * - * <!-- Optional: defaults to "java.log" --> + * <!-- Optional: defaults to {@code "java.log"} --> * <log>application.log</log> * - * <!-- Optional: defaults to "ERROR" --> + * <!-- Optional: defaults to {@code "ERROR"} --> * <flushLevel>WARNING</flushLevel> * - * <!-- Optional: defaults to ASYNC --> + * <!-- Optional: defaults to {@code ASYNC} --> * <writeSynchronicity>SYNC</writeSynchronicity> * + * <!-- Optional: defaults to {@code true} --> + * <autoPopulateMetadata>false</autoPopulateMetadata> + * + * <!-- Optional: defaults to {@code false} --> + * <redirectToStdout>true</redirectToStdout> + * * <!-- Optional: auto detects on App Engine Flex, Standard, GCE and GKE, defaults to "global". See supported resource types --> @@ -94,6 +100,7 @@ public class LoggingAppender extends UnsynchronizedAppenderBase { private volatile Logging logging; private LoggingOptions loggingOptions; + private MonitoredResource monitoredResource; private List loggingEnhancers; private List loggingEventEnhancers; private WriteOption[] defaultWriteOptions; @@ -102,6 +109,8 @@ public class LoggingAppender extends UnsynchronizedAppenderBase { private String log; private String resourceType; private String credentialsFile; + private boolean autoPopulateMetadata = true; + private boolean redirectToStdout = false; private Synchronicity writeSyncFlag = Synchronicity.ASYNC; private final Set enhancerClassNames = new HashSet<>(); private final Set loggingEventEnhancerClassNames = new HashSet<>(); @@ -157,7 +166,7 @@ public void setCredentialsFile(String credentialsFile) { /** * Sets the log ingestion mode. It can be one of the {@link Synchronicity} values. * - *

Default to `Synchronicity.ASYNC`. + *

Default to {@code Synchronicity.ASYNC} * * @param flag the new ingestion mode. */ @@ -165,6 +174,29 @@ public void setWriteSynchronicity(Synchronicity flag) { this.writeSyncFlag = flag; } + /** + * Sets the automatic population of metadata fields for ingested logs. + * + *

Default to {@code true}. + * + * @param flag the metadata auto-population flag. + */ + public void setAutoPopulateMetadata(boolean flag) { + autoPopulateMetadata = flag; + } + + /** + * Sets the redirect of the appender's output to STDOUT instead of ingesting logs to Cloud Logging + * using Logging API. + * + *

Default to {@code false}. + * + * @param flag the redirect flag. + */ + public void setRedirectToStdout(boolean flag) { + redirectToStdout = flag; + } + /** Add extra labels using classes that implement {@link LoggingEnhancer}. */ public void addEnhancer(String enhancerClassName) { this.enhancerClassNames.add(enhancerClassName); @@ -186,6 +218,11 @@ public Synchronicity getWriteSynchronicity() { return (this.writeSyncFlag != null) ? this.writeSyncFlag : Synchronicity.ASYNC; } + @InternalApi("Visible for testing") + void setMonitoredResource(MonitoredResource monitoredResource) { + this.monitoredResource = monitoredResource; + } + private Level getFlushLevel() { return (flushLevel != null) ? flushLevel : Level.ERROR; } @@ -240,9 +277,15 @@ public synchronized void start() { // create new Logging instance // - MonitoredResource resource = getMonitoredResource(getProjectId()); + if (autoPopulateMetadata) { + monitoredResource = getMonitoredResource(getProjectId()); + } else { + monitoredResource = null; + } defaultWriteOptions = - new WriteOption[] {WriteOption.logName(getLogName()), WriteOption.resource(resource)}; + new WriteOption[] { + WriteOption.logName(getLogName()), WriteOption.resource(monitoredResource) + }; Level flushLevel = getFlushLevel(); if (flushLevel != Level.OFF) { getLogging().setFlushSeverity(severityFor(flushLevel)); @@ -263,8 +306,26 @@ String getProjectId() { @Override protected void append(ILoggingEvent e) { - LogEntry logEntry = logEntryFor(e); - getLogging().write(Collections.singleton(logEntry), defaultWriteOptions); + Iterable entries = Collections.singleton(logEntryFor(e)); + if (autoPopulateMetadata) { + entries = + getLogging() + .populateMetadata( + entries, + monitoredResource, + "com.google.cloud.logging", + "jdk", + "sun", + "java", + "ch.qos.logback"); + } + if (redirectToStdout) { + for (LogEntry entry : entries) { + System.out.println(entry.toStructuredJsonString()); + } + } else { + getLogging().write(entries, defaultWriteOptions); + } } @Override @@ -293,6 +354,7 @@ Logging getLogging() { } /** Flushes any pending asynchronous logging writes. */ + @Deprecated public void flush() { if (!isStarted()) { return; @@ -305,15 +367,11 @@ public void flush() { /** Gets the {@link LoggingOptions} to use for this {@link LoggingAppender}. */ protected LoggingOptions getLoggingOptions() { if (loggingOptions == null) { - if (Strings.isNullOrEmpty(credentialsFile)) { - loggingOptions = LoggingOptions.getDefaultInstance(); - } else { + LoggingOptions.Builder builder = LoggingOptions.newBuilder(); + if (!Strings.isNullOrEmpty(credentialsFile)) { try { - loggingOptions = - LoggingOptions.newBuilder() - .setCredentials( - GoogleCredentials.fromStream(new FileInputStream(credentialsFile))) - .build(); + builder.setCredentials( + GoogleCredentials.fromStream(new FileInputStream(credentialsFile))); } catch (IOException e) { throw new RuntimeException( String.format( @@ -322,6 +380,9 @@ protected LoggingOptions getLoggingOptions() { e); } } + // opt-out metadata auto-population to control it in the appender code + builder.setAutoPopulateMetadata(false); + loggingOptions = builder.build(); } return loggingOptions; } diff --git a/src/test/java/com/example/enhancers/AnotherTestLoggingEnhancer.java b/src/test/java/com/example/enhancers/AnotherTestLoggingEnhancer.java deleted file mode 100644 index d9da84471..000000000 --- a/src/test/java/com/example/enhancers/AnotherTestLoggingEnhancer.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2017 Google LLC - * - * 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.example.enhancers; - -import com.google.cloud.logging.LogEntry; -import com.google.cloud.logging.LoggingEnhancer; - -public class AnotherTestLoggingEnhancer implements LoggingEnhancer { - - @Override - public void enhanceLogEntry(LogEntry.Builder logEntry) { - logEntry.addLabel("test-label-2", "test-value-2"); - } -} diff --git a/src/test/java/com/example/enhancers/TestLoggingEnhancer.java b/src/test/java/com/example/enhancers/TestLoggingEnhancer.java deleted file mode 100644 index f982b3f06..000000000 --- a/src/test/java/com/example/enhancers/TestLoggingEnhancer.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2017 Google LLC - * - * 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.example.enhancers; - -import com.google.cloud.logging.LogEntry; -import com.google.cloud.logging.LoggingEnhancer; - -public class TestLoggingEnhancer implements LoggingEnhancer { - - @Override - public void enhanceLogEntry(LogEntry.Builder logEntry) { - logEntry.addLabel("test-label-1", "test-value-1"); - } -} diff --git a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java index 6cb2a01b2..15c7213f3 100644 --- a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java @@ -22,23 +22,26 @@ import static org.easymock.EasyMock.expectLastCall; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; -import static org.junit.Assert.fail; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.filter.ThresholdFilter; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.classic.spi.LoggingEvent; +import com.google.api.client.util.Strings; import com.google.cloud.MonitoredResource; import com.google.cloud.Timestamp; import com.google.cloud.logging.LogEntry; import com.google.cloud.logging.Logging; import com.google.cloud.logging.Logging.WriteOption; +import com.google.cloud.logging.LoggingEnhancer; import com.google.cloud.logging.LoggingOptions; -import com.google.cloud.logging.Payload.JsonPayload; +import com.google.cloud.logging.Payload; import com.google.cloud.logging.Severity; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; import java.time.Instant; -import java.util.HashMap; import java.util.Map; import org.easymock.Capture; import org.easymock.EasyMock; @@ -50,14 +53,79 @@ @RunWith(EasyMockRunner.class) public class LoggingAppenderTest { - private final String projectId = "test-project"; + private static final String PROJECT_ID = "test-project"; + private static Payload.JsonPayload JSON_PAYLOAD = + Payload.JsonPayload.of(ImmutableMap.of("message", "this is a test")); + private static Payload.JsonPayload JSON_ERROR_PAYLOAD = + Payload.JsonPayload.of( + ImmutableMap.of( + "message", + "this is a test", + "@type", + "type.googleapis.com/google.devtools.clouderrorreporting.v1beta1.ReportedErrorEvent")); + private static final MonitoredResource DEFAULT_RESOURCE = + MonitoredResource.of("global", ImmutableMap.of("project_id", PROJECT_ID)); + private static final LogEntry WARN_ENTRY = + LogEntry.newBuilder(JSON_PAYLOAD) + .setTimestamp(Instant.ofEpochMilli(100000L)) + .setSeverity(Severity.WARNING) + .setLabels( + new ImmutableMap.Builder() + .put("levelName", "WARN") + .put("levelValue", String.valueOf(30000L)) + .put("loggerName", LoggingAppenderTest.class.getName()) + // .put("test-label-1", "test-value-1") + // .put("test-label-2", "test-value-2") + .build()) + .build(); + private static final LogEntry ERROR_ENTRY = + LogEntry.newBuilder(JSON_ERROR_PAYLOAD) + .setTimestamp(Instant.ofEpochMilli(100000L)) + .setSeverity(Severity.ERROR) + .setLabels( + new ImmutableMap.Builder() + .put("levelName", "ERROR") + .put("levelValue", String.valueOf(40000L)) + .put("loggerName", LoggingAppenderTest.class.getName()) + .build()) + .build(); + private static final LogEntry INFO_ENTRY = + LogEntry.newBuilder(JSON_PAYLOAD) + .setTimestamp(Instant.ofEpochMilli(100000L)) + .setSeverity(Severity.INFO) + .setLabels( + new ImmutableMap.Builder() + .put("levelName", "INFO") + .put("levelValue", String.valueOf(20000L)) + .put("loggerName", LoggingAppenderTest.class.getName()) + .put("mdc1", "value1") + .put("mdc2", "value2") + .build()) + .build(); + private Logging logging; private LoggingAppender loggingAppender; + static class CustomLoggingEventEnhancer implements LoggingEventEnhancer { + + @Override + public void enhanceLogEntry(LogEntry.Builder builder, ILoggingEvent e) { + builder.addLabel("foo", "bar"); + } + } + + static class CustomLoggingEnhancer implements LoggingEnhancer { + + @Override + public void enhanceLogEntry(LogEntry.Builder builder) { + builder.addLabel("foo", "bar"); + } + } + class TestLoggingAppender extends LoggingAppender { @Override String getProjectId() { - return projectId; + return PROJECT_ID; } @Override @@ -70,6 +138,7 @@ Logging getLogging() { public void setUp() { logging = EasyMock.createStrictMock(Logging.class); loggingAppender = new TestLoggingAppender(); + loggingAppender.setAutoPopulateMetadata(false); } private final WriteOption[] defaultWriteOptions = @@ -78,29 +147,18 @@ public void setUp() { WriteOption.resource( MonitoredResource.newBuilder("global") .setLabels( - new ImmutableMap.Builder().put("project_id", projectId).build()) + new ImmutableMap.Builder() + .put("project_id", PROJECT_ID) + .build()) .build()) }; @Test public void testFlushLevelConfigUpdatesLoggingFlushSeverity() { - Map jsonContent = new HashMap<>(); - jsonContent.put("message", "this is a test"); - JsonPayload payload = JsonPayload.of(jsonContent); - LogEntry logEntry = - LogEntry.newBuilder(payload) - .setTimestamp(Instant.ofEpochMilli(100000L)) - .setSeverity(Severity.WARNING) - .setLabels( - new ImmutableMap.Builder() - .put("levelName", "WARN") - .put("levelValue", String.valueOf(30000L)) - .put("loggerName", this.getClass().getName()) - .build()) - .build(); logging.setFlushSeverity(Severity.WARNING); Capture> capturedArgument = Capture.newInstance(); - logging.write(capture(capturedArgument), (WriteOption) anyObject(), (WriteOption) anyObject()); + logging.write( + capture(capturedArgument), anyObject(WriteOption.class), anyObject(WriteOption.class)); replay(logging); Timestamp timestamp = Timestamp.ofTimeSecondsAndNanos(100000, 0); LoggingEvent loggingEvent = createLoggingEvent(Level.WARN, timestamp.getSeconds()); @@ -110,7 +168,7 @@ public void testFlushLevelConfigUpdatesLoggingFlushSeverity() { loggingAppender.doAppend(loggingEvent); verify(logging); assertThat(capturedArgument.getValue().iterator().hasNext()).isTrue(); - assertThat(capturedArgument.getValue().iterator().next()).isEqualTo(logEntry); + assertThat(capturedArgument.getValue().iterator().next()).isEqualTo(WARN_ENTRY); } @Test @@ -123,26 +181,10 @@ public void testFlushLevelConfigSupportsFlushLevelOff() { @Test public void testFilterLogsOnlyLogsAtOrAboveLogLevel() { - Map jsonContent = new HashMap<>(); - jsonContent.put("message", "this is a test"); - jsonContent.put( - "@type", - "type.googleapis.com/google.devtools.clouderrorreporting.v1beta1.ReportedErrorEvent"); - JsonPayload payload = JsonPayload.of(jsonContent); - LogEntry logEntry = - LogEntry.newBuilder(payload) - .setTimestamp(Instant.ofEpochMilli(100000L)) - .setSeverity(Severity.ERROR) - .setLabels( - new ImmutableMap.Builder() - .put("levelName", "ERROR") - .put("levelValue", String.valueOf(40000L)) - .put("loggerName", this.getClass().getName()) - .build()) - .build(); logging.setFlushSeverity(Severity.ERROR); Capture> capturedArgument = Capture.newInstance(); - logging.write(capture(capturedArgument), (WriteOption) anyObject(), (WriteOption) anyObject()); + logging.write( + capture(capturedArgument), anyObject(WriteOption.class), anyObject(WriteOption.class)); expectLastCall().once(); replay(logging); Timestamp timestamp = Timestamp.ofTimeSecondsAndNanos(100000, 0); @@ -159,41 +201,7 @@ public void testFilterLogsOnlyLogsAtOrAboveLogLevel() { loggingAppender.doAppend(loggingEvent2); verify(logging); assertThat(capturedArgument.getValue().iterator().hasNext()).isTrue(); - assertThat(capturedArgument.getValue().iterator().next()).isEqualTo(logEntry); - } - - @Test - public void testEnhancersAddCorrectLabelsToLogEntries() { - Map jsonContent = new HashMap<>(); - jsonContent.put("message", "this is a test"); - JsonPayload payload = JsonPayload.of(jsonContent); - LogEntry logEntry = - LogEntry.newBuilder(payload) - .setTimestamp(Instant.ofEpochMilli(100000L)) - .setSeverity(Severity.WARNING) - .setLabels( - new ImmutableMap.Builder() - .put("levelName", "WARN") - .put("levelValue", String.valueOf(30000L)) - .put("loggerName", this.getClass().getName()) - .put("test-label-1", "test-value-1") - .put("test-label-2", "test-value-2") - .build()) - .build(); - logging.setFlushSeverity(Severity.ERROR); - Capture> capturedArgument = Capture.newInstance(); - logging.write(capture(capturedArgument), (WriteOption) anyObject(), (WriteOption) anyObject()); - expectLastCall().once(); - replay(logging); - loggingAppender.addEnhancer("com.example.enhancers.TestLoggingEnhancer"); - loggingAppender.addEnhancer("com.example.enhancers.AnotherTestLoggingEnhancer"); - loggingAppender.start(); - Timestamp timestamp = Timestamp.ofTimeSecondsAndNanos(100000, 0); - LoggingEvent loggingEvent = createLoggingEvent(Level.WARN, timestamp.getSeconds()); - loggingAppender.doAppend(loggingEvent); - verify(logging); - assertThat(capturedArgument.getValue().iterator().hasNext()).isTrue(); - assertThat(capturedArgument.getValue().iterator().next()).isEqualTo(logEntry); + assertThat(capturedArgument.getValue().iterator().next()).isEqualTo(ERROR_ENTRY); } @Test @@ -217,25 +225,10 @@ public void testDefaultWriteOptionsHasExpectedDefaults() { @Test public void testMdcValuesAreConvertedToLabels() { - Map jsonContent = new HashMap<>(); - jsonContent.put("message", "this is a test"); - JsonPayload payload = JsonPayload.of(jsonContent); - LogEntry logEntry = - LogEntry.newBuilder(payload) - .setTimestamp(Instant.ofEpochMilli(100000L)) - .setSeverity(Severity.INFO) - .setLabels( - new ImmutableMap.Builder() - .put("levelName", "INFO") - .put("levelValue", String.valueOf(20000L)) - .put("loggerName", this.getClass().getName()) - .put("mdc1", "value1") - .put("mdc2", "value2") - .build()) - .build(); logging.setFlushSeverity(Severity.ERROR); Capture> capturedArgument = Capture.newInstance(); - logging.write(capture(capturedArgument), (WriteOption) anyObject(), (WriteOption) anyObject()); + logging.write( + capture(capturedArgument), anyObject(WriteOption.class), anyObject(WriteOption.class)); expectLastCall().once(); replay(logging); Timestamp timestamp = Timestamp.ofTimeSecondsAndNanos(100000, 0); @@ -246,33 +239,23 @@ public void testMdcValuesAreConvertedToLabels() { loggingAppender.doAppend(loggingEvent); verify(logging); assertThat(capturedArgument.getValue().iterator().hasNext()).isTrue(); - assertThat(capturedArgument.getValue().iterator().next()).isEqualTo(logEntry); + assertThat(capturedArgument.getValue().iterator().next()).isEqualTo(INFO_ENTRY); } - @Test - public void testCreateLoggingOptions() { - // Try to build LoggingOptions with custom credentials. + @Test(expected = RuntimeException.class) + public void testCreateLoggingOptionsWithInvalidCredentials() { final String nonExistentFile = "/path/to/non/existent/file"; LoggingAppender appender = new LoggingAppender(); appender.setCredentialsFile(nonExistentFile); - try { - appender.getLoggingOptions(); - fail("Expected exception"); - } catch (Exception e) { - assertThat(e.getMessage().contains(nonExistentFile)); - } - // Try to build LoggingOptions with default credentials. - LoggingOptions defaultOptions = null; - try { - defaultOptions = LoggingOptions.getDefaultInstance(); - } catch (Exception e) { - // Could not build a default LoggingOptions instance. - } - if (defaultOptions != null) { - appender = new LoggingAppender(); - LoggingOptions options = appender.getLoggingOptions(); - assertThat(options).isEqualTo(defaultOptions); - } + appender.getLoggingOptions(); + } + + @Test + public void testCreateWithDefaultLoggingOptions() { + LoggingOptions defaultOptions = LoggingOptions.getDefaultInstance(); + LoggingAppender appender = new LoggingAppender(); + LoggingOptions options = appender.getLoggingOptions(); + assertThat(options).isEqualTo(defaultOptions); } private LoggingEvent createLoggingEvent(Level level, long timestamp) { @@ -291,7 +274,8 @@ public void testMdcValuesAreConvertedToLabelsWithPassingNullValues() { MDC.put("mdc3", "value3"); logging.setFlushSeverity(Severity.ERROR); Capture> capturedArgument = Capture.newInstance(); - logging.write(capture(capturedArgument), (WriteOption) anyObject(), (WriteOption) anyObject()); + logging.write( + capture(capturedArgument), anyObject(WriteOption.class), anyObject(WriteOption.class)); expectLastCall().once(); replay(logging); Timestamp timestamp = Timestamp.ofTimeSecondsAndNanos(100000, 0); @@ -313,13 +297,13 @@ public void testAddCustomLoggingEventEnhancers() { MDC.put("mdc1", "value1"); logging.setFlushSeverity(Severity.ERROR); Capture> capturedArgument = Capture.newInstance(); - logging.write(capture(capturedArgument), (WriteOption) anyObject(), (WriteOption) anyObject()); + logging.write( + capture(capturedArgument), anyObject(WriteOption.class), anyObject(WriteOption.class)); expectLastCall().once(); replay(logging); Timestamp timestamp = Timestamp.ofTimeSecondsAndNanos(100000, 0); LoggingEvent loggingEvent = createLoggingEvent(Level.INFO, timestamp.getSeconds()); - loggingAppender.addLoggingEventEnhancer(CustomLoggingEventEnhancer1.class.getName()); - loggingAppender.addLoggingEventEnhancer(CustomLoggingEventEnhancer2.class.getName()); + loggingAppender.addLoggingEventEnhancer(CustomLoggingEventEnhancer.class.getName()); loggingAppender.start(); loggingAppender.doAppend(loggingEvent); verify(logging); @@ -327,15 +311,36 @@ public void testAddCustomLoggingEventEnhancers() { Map capturedArgumentMap = capturedArgument.getValue().iterator().next().getLabels(); assertThat(capturedArgumentMap.get("mdc1")).isNull(); - assertThat(capturedArgumentMap.get("foo")).isEqualTo("foo"); - assertThat(capturedArgumentMap.get("bar")).isEqualTo("bar"); + assertThat(capturedArgumentMap.get("foo")).isEqualTo("bar"); } @Test - public void testFlush() { + public void testAddCustomLoggingEnhancer() { logging.setFlushSeverity(Severity.ERROR); Capture> capturedArgument = Capture.newInstance(); - logging.write(capture(capturedArgument), (WriteOption) anyObject(), (WriteOption) anyObject()); + logging.write( + capture(capturedArgument), anyObject(WriteOption.class), anyObject(WriteOption.class)); + expectLastCall().once(); + replay(logging); + loggingAppender.addEnhancer(CustomLoggingEnhancer.class.getName()); + loggingAppender.start(); + Timestamp timestamp = Timestamp.ofTimeSecondsAndNanos(100000, 0); + LoggingEvent loggingEvent = createLoggingEvent(Level.WARN, timestamp.getSeconds()); + loggingAppender.doAppend(loggingEvent); + verify(logging); + Map capturedArgumentMap = + capturedArgument.getValue().iterator().next().getLabels(); + assertThat(capturedArgumentMap.get("foo")).isEqualTo("bar"); + } + + @Test + @SuppressWarnings("deprecation") + public void testFlush() { + logging.setFlushSeverity(Severity.ERROR); + logging.write( + EasyMock.>anyObject(), + anyObject(WriteOption.class), + anyObject(WriteOption.class)); expectLastCall().times(2); logging.flush(); replay(logging); @@ -349,19 +354,84 @@ public void testFlush() { verify(logging); } - static class CustomLoggingEventEnhancer1 implements LoggingEventEnhancer { + @Test + public void testAutoPopulationEnabled() { + logging.setFlushSeverity(Severity.ERROR); + Capture> capturedLogEntries = Capture.newInstance(); + EasyMock.expect( + logging.populateMetadata( + capture(capturedLogEntries), + EasyMock.eq(DEFAULT_RESOURCE), + EasyMock.eq("com.google.cloud.logging"), + EasyMock.eq("jdk"), + EasyMock.eq("sun"), + EasyMock.eq("java"), + EasyMock.eq("ch.qos.logback"))) + .andReturn(ImmutableList.of(INFO_ENTRY)) + .once(); + // it is impossible to define expectation for varargs using a single anyObject() matcher + // see the EasyMock bug https://github.com/easymock/easymock/issues/130. + // the following mock uses the known fact that the method pass two WriteOption arguments + // the arguments should be replaced with a single anyObject() matchers when the bug is fixed + logging.write( + EasyMock.>anyObject(), + anyObject(WriteOption.class), + anyObject(WriteOption.class)); + expectLastCall().once(); + replay(logging); - @Override - public void enhanceLogEntry(LogEntry.Builder builder, ILoggingEvent e) { - builder.addLabel("foo", "foo"); - } + loggingAppender.setMonitoredResource(DEFAULT_RESOURCE); + loggingAppender.setAutoPopulateMetadata(true); + loggingAppender.start(); + Timestamp timestamp = Timestamp.ofTimeSecondsAndNanos(100000, 0); + LoggingEvent loggingEvent = createLoggingEvent(Level.INFO, timestamp.getSeconds()); + loggingEvent.setMDCPropertyMap(ImmutableMap.of("mdc1", "value1", "mdc2", "value2")); + loggingAppender.doAppend(loggingEvent); + verify(logging); + LogEntry testLogEntry = capturedLogEntries.getValue().iterator().next(); + assertThat(testLogEntry).isEqualTo(INFO_ENTRY); } - static class CustomLoggingEventEnhancer2 implements LoggingEventEnhancer { + @Test + public void testRedirectToStdoutEnabled() { + logging.setFlushSeverity(Severity.ERROR); + EasyMock.expect( + logging.populateMetadata( + EasyMock.>anyObject(), + EasyMock.anyObject(MonitoredResource.class), + EasyMock.anyString(), + EasyMock.anyString(), + EasyMock.anyString(), + EasyMock.anyString(), + EasyMock.anyString())) + .andReturn(ImmutableList.of(INFO_ENTRY)) + .once(); + replay(logging); - @Override - public void enhanceLogEntry(LogEntry.Builder builder, ILoggingEvent e) { - builder.addLabel("bar", "bar"); - } + ByteArrayOutputStream bout = new ByteArrayOutputStream(); + PrintStream out = new PrintStream(bout); + System.setOut(out); + loggingAppender.setMonitoredResource(DEFAULT_RESOURCE); + loggingAppender.setAutoPopulateMetadata(true); + loggingAppender.setRedirectToStdout(true); + loggingAppender.start(); + Timestamp timestamp = Timestamp.ofTimeSecondsAndNanos(100000, 0); + LoggingEvent loggingEvent = createLoggingEvent(Level.INFO, timestamp.getSeconds()); + loggingAppender.doAppend(loggingEvent); + verify(logging); + assertThat(Strings.isNullOrEmpty(bout.toString())).isFalse(); + System.setOut(null); + } + + @Test + public void testRedirectToStdoutDisabled() { + ByteArrayOutputStream bout = new ByteArrayOutputStream(); + PrintStream out = new PrintStream(bout); + System.setOut(out); + + testAutoPopulationEnabled(); + + assertThat(Strings.isNullOrEmpty(bout.toString())).isTrue(); + System.setOut(null); } } From f54747ac9df4763215e31538cefa87663c327c8d Mon Sep 17 00:00:00 2001 From: minherz Date: Wed, 12 Jan 2022 11:39:31 +0000 Subject: [PATCH 4/8] chore: remove testing default option from unit tests to test it as unit test the call LoggingOptions.getDefaultInstance() should be mocked. --- .../google/cloud/logging/logback/LoggingAppenderTest.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java index 15c7213f3..decfc442d 100644 --- a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java @@ -250,14 +250,6 @@ public void testCreateLoggingOptionsWithInvalidCredentials() { appender.getLoggingOptions(); } - @Test - public void testCreateWithDefaultLoggingOptions() { - LoggingOptions defaultOptions = LoggingOptions.getDefaultInstance(); - LoggingAppender appender = new LoggingAppender(); - LoggingOptions options = appender.getLoggingOptions(); - assertThat(options).isEqualTo(defaultOptions); - } - private LoggingEvent createLoggingEvent(Level level, long timestamp) { LoggingEvent loggingEvent = new LoggingEvent(); loggingEvent.setMessage("this is a test"); From a040191c1fad77e8797dbb8fda0f1026d7e87274 Mon Sep 17 00:00:00 2001 From: minherz Date: Wed, 12 Jan 2022 11:56:42 +0000 Subject: [PATCH 5/8] chore: fix the lint --- .../com/google/cloud/logging/logback/LoggingAppenderTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java index decfc442d..bdb283223 100644 --- a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java @@ -34,7 +34,6 @@ import com.google.cloud.logging.Logging; import com.google.cloud.logging.Logging.WriteOption; import com.google.cloud.logging.LoggingEnhancer; -import com.google.cloud.logging.LoggingOptions; import com.google.cloud.logging.Payload; import com.google.cloud.logging.Severity; import com.google.common.collect.ImmutableList; From 3ea914b93f6ab58c89fa7908b33f19bcd8994131 Mon Sep 17 00:00:00 2001 From: minherz Date: Thu, 13 Jan 2022 08:04:11 +0000 Subject: [PATCH 6/8] chore: fix dependency error --- .../com/google/cloud/logging/logback/LoggingAppenderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java index bdb283223..487c40aa7 100644 --- a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java @@ -27,7 +27,7 @@ import ch.qos.logback.classic.filter.ThresholdFilter; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.classic.spi.LoggingEvent; -import com.google.api.client.util.Strings; +import com.google.common.base.Strings; import com.google.cloud.MonitoredResource; import com.google.cloud.Timestamp; import com.google.cloud.logging.LogEntry; From b9364d0bedb1349b9e36d69eaa4d8e543d84eac8 Mon Sep 17 00:00:00 2001 From: minherz Date: Thu, 13 Jan 2022 08:52:02 +0000 Subject: [PATCH 7/8] chore: minor lint fix --- .../com/google/cloud/logging/logback/LoggingAppenderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java index 487c40aa7..ca3986e6d 100644 --- a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java @@ -27,7 +27,6 @@ import ch.qos.logback.classic.filter.ThresholdFilter; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.classic.spi.LoggingEvent; -import com.google.common.base.Strings; import com.google.cloud.MonitoredResource; import com.google.cloud.Timestamp; import com.google.cloud.logging.LogEntry; @@ -36,6 +35,7 @@ import com.google.cloud.logging.LoggingEnhancer; import com.google.cloud.logging.Payload; import com.google.cloud.logging.Severity; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import java.io.ByteArrayOutputStream; From a24f6206ec54708b91805790c4ae3382234af666 Mon Sep 17 00:00:00 2001 From: minherz Date: Sun, 16 Jan 2022 12:51:41 +0000 Subject: [PATCH 8/8] chore: refactoring setup monitored resource --- .../logging/logback/LoggingAppender.java | 23 +++++++++---------- .../logging/logback/LoggingAppenderTest.java | 4 ++-- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java b/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java index 61cbd3dae..6cdb6a13a 100644 --- a/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java +++ b/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java @@ -218,8 +218,16 @@ public Synchronicity getWriteSynchronicity() { return (this.writeSyncFlag != null) ? this.writeSyncFlag : Synchronicity.ASYNC; } + private void setupMonitoredResource() { + if (autoPopulateMetadata) { + monitoredResource = MonitoredResourceUtil.getResource(getProjectId(), resourceType); + } else { + monitoredResource = null; + } + } + @InternalApi("Visible for testing") - void setMonitoredResource(MonitoredResource monitoredResource) { + void setupMonitoredResource(MonitoredResource monitoredResource) { this.monitoredResource = monitoredResource; } @@ -231,10 +239,6 @@ private String getLogName() { return (log != null) ? log : "java.log"; } - private MonitoredResource getMonitoredResource(String projectId) { - return MonitoredResourceUtil.getResource(projectId, resourceType); - } - private List getLoggingEnhancers() { return getEnhancers(enhancerClassNames, LoggingEnhancer.class); } @@ -275,13 +279,8 @@ public synchronized void start() { return; } - // create new Logging instance - // - if (autoPopulateMetadata) { - monitoredResource = getMonitoredResource(getProjectId()); - } else { - monitoredResource = null; - } + setupMonitoredResource(); + defaultWriteOptions = new WriteOption[] { WriteOption.logName(getLogName()), WriteOption.resource(monitoredResource) diff --git a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java index ca3986e6d..1864ee9c9 100644 --- a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java @@ -371,7 +371,7 @@ public void testAutoPopulationEnabled() { expectLastCall().once(); replay(logging); - loggingAppender.setMonitoredResource(DEFAULT_RESOURCE); + loggingAppender.setupMonitoredResource(DEFAULT_RESOURCE); loggingAppender.setAutoPopulateMetadata(true); loggingAppender.start(); Timestamp timestamp = Timestamp.ofTimeSecondsAndNanos(100000, 0); @@ -402,7 +402,7 @@ public void testRedirectToStdoutEnabled() { ByteArrayOutputStream bout = new ByteArrayOutputStream(); PrintStream out = new PrintStream(bout); System.setOut(out); - loggingAppender.setMonitoredResource(DEFAULT_RESOURCE); + loggingAppender.setupMonitoredResource(DEFAULT_RESOURCE); loggingAppender.setAutoPopulateMetadata(true); loggingAppender.setRedirectToStdout(true); loggingAppender.start();