From 1cc4cfa471e94381bc27e71099c4ddaae26a054e Mon Sep 17 00:00:00 2001 From: losalex Date: Mon, 27 Jun 2022 13:53:51 -0700 Subject: [PATCH 1/4] feat: Add support for library instrumentation --- .../logging/logback/LoggingAppender.java | 29 ++++++++ .../logging/logback/LoggingAppenderTest.java | 73 +++++++++++++++++++ 2 files changed, 102 insertions(+) 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 a8b751077..494cc9245 100644 --- a/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java +++ b/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java @@ -25,6 +25,7 @@ import com.google.api.core.InternalApi; import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.MonitoredResource; +import com.google.cloud.logging.Instrumentation; import com.google.cloud.logging.LogEntry; import com.google.cloud.logging.Logging; import com.google.cloud.logging.Logging.WriteOption; @@ -100,6 +101,9 @@ public class LoggingAppender extends UnsynchronizedAppenderBase { "type.googleapis.com/google.devtools.clouderrorreporting.v1beta1.ReportedErrorEvent"; private static final List DEFAULT_LOGGING_EVENT_ENHANCERS = ImmutableList.of(new MDCEventEnhancer()); + public static final String JAVA_LOGBACK_LIBRARY_NAME = "java-logback"; + private static boolean instrumentationAdded = false; + private static Object instrumentationLock = new Object(); private volatile Logging logging; private LoggingOptions loggingOptions; @@ -318,6 +322,16 @@ String getProjectId() { @Override protected void append(ILoggingEvent e) { Iterable entries = Collections.singleton(logEntryFor(e)); + // Check if instrumentation was already added - if not, create a log entry with instrumentation + // data + if (!setInstrumentationStatus(true)) { + List result = new ArrayList(); + entries.forEach(result::add); + result.add( + Instrumentation.createDiagnosticEntry( + JAVA_LOGBACK_LIBRARY_NAME, Instrumentation.getLibraryVersion(LoggingAppender.class))); + entries = result; + } if (autoPopulateMetadata) { entries = getLogging() @@ -490,4 +504,19 @@ private static Severity severityFor(Level level) { return Severity.DEFAULT; } } + + /** + * The package-private helper method used to set the flag which indicates if instrumentation info + * already written or not. + * + * @returns The value of the flag before it was set. + */ + static boolean setInstrumentationStatus(boolean value) { + if (instrumentationAdded == value) return instrumentationAdded; + synchronized (instrumentationLock) { + boolean current = instrumentationAdded; + instrumentationAdded = value; + return current; + } + } } 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 b71747af3..f1bdbebf4 100644 --- a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java @@ -22,6 +22,7 @@ import static org.easymock.EasyMock.expectLastCall; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertEquals; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.filter.ThresholdFilter; @@ -29,15 +30,20 @@ import ch.qos.logback.classic.spi.LoggingEvent; import com.google.cloud.MonitoredResource; import com.google.cloud.Timestamp; +import com.google.cloud.logging.Instrumentation; 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.Payload; +import com.google.cloud.logging.Payload.JsonPayload; +import com.google.cloud.logging.Payload.Type; 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 com.google.protobuf.ListValue; +import com.google.protobuf.Value; import java.io.ByteArrayOutputStream; import java.io.PrintStream; import java.time.Instant; @@ -139,6 +145,7 @@ Logging getLogging() { @Before public void setUp() { + LoggingAppender.setInstrumentationStatus(true); logging = EasyMock.createStrictMock(Logging.class); loggingAppender = new TestLoggingAppender(); loggingAppender.setAutoPopulateMetadata(false); @@ -446,4 +453,70 @@ public void testRedirectToStdoutDisabled() { assertThat(Strings.isNullOrEmpty(bout.toString())).isTrue(); System.setOut(null); } + + @Test + public void testFDiagnosticInfoAdded() { + LoggingAppender.setInstrumentationStatus(false); + Capture> capturedArgument = Capture.newInstance(); + logging.setFlushSeverity(Severity.ERROR); + logging.write( + capture(capturedArgument), anyObject(WriteOption.class), anyObject(WriteOption.class)); + replay(logging); + LoggingEvent loggingEvent = + createLoggingEvent(Level.ERROR, Timestamp.ofTimeSecondsAndNanos(100000, 0).getSeconds()); + loggingAppender.start(); + loggingAppender.doAppend(loggingEvent); + verify(logging); + int count = 0; + int diagnosticRecordCount = 0; + for (LogEntry entry : capturedArgument.getValue()) { + count++; + if (entry.getPayload().getType() == Type.JSON) { + JsonPayload payload = entry.getPayload(); + if (!payload.getData().containsFields(Instrumentation.DIAGNOSTIC_INFO_KEY)) continue; + ListValue infoList = + payload + .getData() + .getFieldsOrThrow(Instrumentation.DIAGNOSTIC_INFO_KEY) + .getStructValue() + .getFieldsOrThrow(Instrumentation.INSTRUMENTATION_SOURCE_KEY) + .getListValue(); + for (Value val : infoList.getValuesList()) { + String name = + val.getStructValue() + .getFieldsOrThrow(Instrumentation.INSTRUMENTATION_NAME_KEY) + .getStringValue(); + assertThat(name.startsWith(Instrumentation.JAVA_LIBRARY_NAME_PREFIX)).isTrue(); + if (name.equals(LoggingAppender.JAVA_LOGBACK_LIBRARY_NAME)) { + diagnosticRecordCount++; + } + } + } + } + assertEquals(count, 2); + assertEquals(diagnosticRecordCount, 1); + } + + @Test + public void testFDiagnosticInfoNotAdded() { + logging.setFlushSeverity(Severity.ERROR); + Capture> capturedArgument = Capture.newInstance(); + logging.write( + capture(capturedArgument), anyObject(WriteOption.class), anyObject(WriteOption.class)); + replay(logging); + LoggingEvent loggingEvent = + createLoggingEvent(Level.WARN, Timestamp.ofTimeSecondsAndNanos(100000, 0).getSeconds()); + loggingAppender.start(); + loggingAppender.doAppend(loggingEvent); + verify(logging); + int count = 0; + for (LogEntry entry : capturedArgument.getValue()) { + count++; + if (entry.getPayload().getType() == Type.JSON) { + JsonPayload payload = entry.getPayload(); + assertThat(payload.getData().containsFields(Instrumentation.DIAGNOSTIC_INFO_KEY)).isFalse(); + } + } + assertEquals(count, 1); + } } From 13520a3df10d82aa8ff4b91504695652160ef32c Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Mon, 27 Jun 2022 20:58:15 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index e02f4c385..1a877aae0 100644 --- a/README.md +++ b/README.md @@ -22,20 +22,20 @@ If you are using Maven, add this to your pom.xml file: com.google.cloud google-cloud-logging-logback - 0.125.2-alpha + 0.125.3-alpha ``` If you are using Gradle without BOM, add this to your dependencies ```Groovy -implementation 'com.google.cloud:google-cloud-logging-logback:0.125.2-alpha' +implementation 'com.google.cloud:google-cloud-logging-logback:0.125.3-alpha' ``` If you are using SBT, add this to your dependencies ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-logging-logback" % "0.125.2-alpha" +libraryDependencies += "com.google.cloud" % "google-cloud-logging-logback" % "0.125.3-alpha" ``` ## Authentication From 6cae2235564e88a1546aab5a085167aff1056694 Mon Sep 17 00:00:00 2001 From: losalex Date: Mon, 27 Jun 2022 14:54:44 -0700 Subject: [PATCH 3/4] Update pom.xml --- pom.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pom.xml b/pom.xml index 0ac4a4dfb..ea512b33a 100644 --- a/pom.xml +++ b/pom.xml @@ -134,6 +134,10 @@ + + com.google.protobuf + protobuf-java + From f83fa07ec4b4a20402c1451c45f68ae4bd525080 Mon Sep 17 00:00:00 2001 From: losalex Date: Tue, 28 Jun 2022 09:50:57 -0700 Subject: [PATCH 4/4] Address PR comments --- .../google/cloud/logging/logback/LoggingAppender.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 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 494cc9245..2ac038058 100644 --- a/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java +++ b/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java @@ -41,7 +41,6 @@ import java.io.IOException; import java.time.Instant; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -321,17 +320,16 @@ String getProjectId() { @Override protected void append(ILoggingEvent e) { - Iterable entries = Collections.singleton(logEntryFor(e)); + List entriesList = new ArrayList(); + entriesList.add(logEntryFor(e)); // Check if instrumentation was already added - if not, create a log entry with instrumentation // data if (!setInstrumentationStatus(true)) { - List result = new ArrayList(); - entries.forEach(result::add); - result.add( + entriesList.add( Instrumentation.createDiagnosticEntry( JAVA_LOGBACK_LIBRARY_NAME, Instrumentation.getLibraryVersion(LoggingAppender.class))); - entries = result; } + Iterable entries = entriesList; if (autoPopulateMetadata) { entries = getLogging()