From 9565ccd52c5e3cde58fea25f4e4ab38270d21ec9 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Sat, 23 Mar 2019 08:29:52 +0100 Subject: [PATCH 1/5] #3215 allow user to specify custom LoggingOptions --- .../google-cloud-logging-logback/README.md | 17 +++++++++++++ .../logging/logback/LoggingAppender.java | 25 +++++++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/README.md b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/README.md index ba815c51741e..54fc6a62aa75 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/README.md +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/README.md @@ -88,6 +88,23 @@ Authentication See the [Authentication](https://github.com/googleapis/google-cloud-java#authentication) section in the base directory's README. +You can also specify custom credentials and other options by creating a subclass of `com.google.cloud.logging.logback.LoggingAppender` and override the method `createLoggingOptions()`. Your logback.xml configuration file must reference your subclass instead of `com.google.cloud.logging.logback.LoggingAppender`. + +```java +public class CustomLoggingAppender extends LoggingAppender { + @Override + public LoggingOptions createLoggingOptions() { + try { + return LoggingOptions.newBuilder() + .setCredentials(GoogleCredentials.fromStream( + new FileInputStream("/path/to/credentials.json"))) + .build(); + } catch (IOException e) { + throw new RuntimeException("Could not find credentials", e); + } + } +} +``` Limitations ----------- diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java index 23d5e4183fdf..ab92cccdf092 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java @@ -38,6 +38,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import javax.annotation.Nonnull; /** * Logback appender for StackDriver Cloud Logging. @@ -60,6 +61,10 @@ *
  • <enhancer>com.example.enhancer2</enhancer> *
  • </appender> * + * + * Users can extend this class and override the method {@link #createLoggingOptions()} if you need + * to specify custom {@link LoggingOptions}, and reference the subclass in logback.xml instead of + * {@link LoggingAppender}. */ public class LoggingAppender extends UnsynchronizedAppenderBase { @@ -186,7 +191,7 @@ public synchronized void start() { } String getProjectId() { - return LoggingOptions.getDefaultInstance().getProjectId(); + return createLoggingOptions().getProjectId(); } @Override @@ -212,13 +217,29 @@ Logging getLogging() { if (logging == null) { synchronized (this) { if (logging == null) { - logging = LoggingOptions.getDefaultInstance().getService(); + logging = createLoggingOptions().getService(); } } } return logging; } + /** + * Creates the {@link LoggingOptions} to use for this {@link LoggingAppender}. Users can subclass + * {@link LoggingAppender} and override this method if they need to supply custom {@link + * LoggingOptions}, such as a specific credentials file. The default implementation returns {@link + * LoggingOptions#getDefaultInstance()}. Make sure to reference your own subclass in your logback + * configuration instead of {@link LoggingAppender} if you want to use custom {@link + * LoggingOptions}. + * + * @return the {@link LoggingOptions} that should be used for this {@link LoggingAppender}. May + * not be null. + */ + @Nonnull + public LoggingOptions createLoggingOptions() { + return LoggingOptions.getDefaultInstance(); + } + private LogEntry logEntryFor(ILoggingEvent e) { StringBuilder payload = new StringBuilder(e.getFormattedMessage()).append('\n'); writeStack(e.getThrowableProxy(), "", payload); From 58ad82ba20c7b2392c67a450a932c7a0827387f9 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Sat, 23 Mar 2019 12:01:22 +0100 Subject: [PATCH 2/5] added test case --- .../logging/logback/LoggingAppenderTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java index 5855c80177d7..b465f3ab73c9 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java @@ -31,6 +31,7 @@ import com.google.cloud.logging.LogEntry; import com.google.cloud.logging.Logging; import com.google.cloud.logging.Logging.WriteOption; +import com.google.cloud.logging.LoggingOptions; import com.google.cloud.logging.Payload.StringPayload; import com.google.cloud.logging.Severity; import com.google.common.collect.ImmutableMap; @@ -211,6 +212,20 @@ public void testMdcValuesAreConvertedToLabels() { assertThat(capturedArgument.getValue().iterator().next()).isEqualTo(logEntry); } + @Test + public void testCreateLoggingOptions() { + LoggingAppender appender = + new LoggingAppender() { + @Override + public LoggingOptions createLoggingOptions() { + return LoggingOptions.newBuilder().setProjectId(projectId).build(); + } + }; + assertThat(appender.getProjectId()).isEqualTo(projectId); + appender.start(); + assertThat(appender.isStarted()).isTrue(); + } + private LoggingEvent createLoggingEvent(Level level, long timestamp) { LoggingEvent loggingEvent = new LoggingEvent(); loggingEvent.setMessage("this is a test"); From d36f0db76f5179afb8ca5dd5637ccbe048e2f3c3 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Wed, 27 Mar 2019 14:57:15 +0100 Subject: [PATCH 3/5] changed implementation to use setter instead of subclass --- .../logging/logback/LoggingAppender.java | 54 ++++++++++++------- .../logging/logback/LoggingAppenderTest.java | 21 ++++---- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java index ab92cccdf092..aafc4ab6bdd5 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java @@ -23,6 +23,7 @@ import ch.qos.logback.core.UnsynchronizedAppenderBase; import ch.qos.logback.core.util.Loader; import com.google.api.core.InternalApi; +import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.MonitoredResource; import com.google.cloud.logging.LogEntry; import com.google.cloud.logging.Logging; @@ -32,13 +33,15 @@ import com.google.cloud.logging.MonitoredResourceUtil; import com.google.cloud.logging.Payload; import com.google.cloud.logging.Severity; +import com.google.common.base.Strings; +import java.io.FileInputStream; +import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import javax.annotation.Nonnull; /** * Logback appender for StackDriver Cloud Logging. @@ -56,15 +59,13 @@ * Standard, GCE and GKE, defaults to "global". See supported resource * types + *
  • <credentialsFile>/path/to/credentials/file</credentialsFile> (Optional, + * defaults to the default credentials of the environment) *
  • (Optional) add custom labels to log entries using {@link LoggingEnhancer} classes. *
  • <enhancer>com.example.enhancer1</enhancer> *
  • <enhancer>com.example.enhancer2</enhancer> *
  • </appender> * - * - * Users can extend this class and override the method {@link #createLoggingOptions()} if you need - * to specify custom {@link LoggingOptions}, and reference the subclass in logback.xml instead of - * {@link LoggingAppender}. */ public class LoggingAppender extends UnsynchronizedAppenderBase { @@ -79,6 +80,7 @@ public class LoggingAppender extends UnsynchronizedAppenderBase { private Level flushLevel; private String log; private String resourceType; + private String credentialsFile; private Set enhancerClassNames = new HashSet<>(); private Set loggingEventEnhancerClassNames = new HashSet<>(); @@ -116,6 +118,17 @@ 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. + * + * @param credentialsFile The credentials file to use. + */ + public void setCredentialsFile(String credentialsFile) { + this.credentialsFile = credentialsFile; + } + /** Add extra labels using classes that implement {@link LoggingEnhancer}. */ public void addEnhancer(String enhancerClassName) { this.enhancerClassNames.add(enhancerClassName); @@ -224,20 +237,23 @@ Logging getLogging() { return logging; } - /** - * Creates the {@link LoggingOptions} to use for this {@link LoggingAppender}. Users can subclass - * {@link LoggingAppender} and override this method if they need to supply custom {@link - * LoggingOptions}, such as a specific credentials file. The default implementation returns {@link - * LoggingOptions#getDefaultInstance()}. Make sure to reference your own subclass in your logback - * configuration instead of {@link LoggingAppender} if you want to use custom {@link - * LoggingOptions}. - * - * @return the {@link LoggingOptions} that should be used for this {@link LoggingAppender}. May - * not be null. - */ - @Nonnull - public LoggingOptions createLoggingOptions() { - return LoggingOptions.getDefaultInstance(); + /** Creates the {@link LoggingOptions} to use for this {@link LoggingAppender}. */ + LoggingOptions createLoggingOptions() { + if (Strings.isNullOrEmpty(credentialsFile)) { + return LoggingOptions.getDefaultInstance(); + } else { + try { + return LoggingOptions.newBuilder() + .setCredentials(GoogleCredentials.fromStream(new FileInputStream(credentialsFile))) + .build(); + } catch (IOException e) { + throw new RuntimeException( + String.format( + "Could not read credentials file %s. Please verify that the file exists and is a valid Google credentials file.", + credentialsFile), + e); + } + } } private LogEntry logEntryFor(ILoggingEvent e) { diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java index b465f3ab73c9..06a485d01aec 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/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.fail; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.filter.ThresholdFilter; @@ -31,7 +32,6 @@ import com.google.cloud.logging.LogEntry; import com.google.cloud.logging.Logging; import com.google.cloud.logging.Logging.WriteOption; -import com.google.cloud.logging.LoggingOptions; import com.google.cloud.logging.Payload.StringPayload; import com.google.cloud.logging.Severity; import com.google.common.collect.ImmutableMap; @@ -214,16 +214,15 @@ public void testMdcValuesAreConvertedToLabels() { @Test public void testCreateLoggingOptions() { - LoggingAppender appender = - new LoggingAppender() { - @Override - public LoggingOptions createLoggingOptions() { - return LoggingOptions.newBuilder().setProjectId(projectId).build(); - } - }; - assertThat(appender.getProjectId()).isEqualTo(projectId); - appender.start(); - assertThat(appender.isStarted()).isTrue(); + final String nonExistentFile = "/path/to/non/existent/file"; + LoggingAppender appender = new LoggingAppender(); + appender.setCredentialsFile(nonExistentFile); + try { + appender.createLoggingOptions(); + fail("Expected exception"); + } catch (Exception e) { + assertThat(e.getMessage().contains(nonExistentFile)); + } } private LoggingEvent createLoggingEvent(Level level, long timestamp) { From 37c07acefa1761f2201b9bfff90957c200d1a04b Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Thu, 28 Mar 2019 09:40:22 +0100 Subject: [PATCH 4/5] cache the created LoggingOptions instead of re-creating every time --- .../logging/logback/LoggingAppender.java | 40 +++++++++++-------- .../logging/logback/LoggingAppenderTest.java | 16 +++++++- 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java index aafc4ab6bdd5..2e49e8d51e9a 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java @@ -73,6 +73,7 @@ public class LoggingAppender extends UnsynchronizedAppenderBase { private static final String LEVEL_VALUE_KEY = "levelValue"; private volatile Logging logging; + private LoggingOptions loggingOptions; private List loggingEnhancers; private List loggingEventEnhancers; private WriteOption[] defaultWriteOptions; @@ -204,7 +205,7 @@ public synchronized void start() { } String getProjectId() { - return createLoggingOptions().getProjectId(); + return getLoggingOptions().getProjectId(); } @Override @@ -230,30 +231,35 @@ Logging getLogging() { if (logging == null) { synchronized (this) { if (logging == null) { - logging = createLoggingOptions().getService(); + logging = getLoggingOptions().getService(); } } } return logging; } - /** Creates the {@link LoggingOptions} to use for this {@link LoggingAppender}. */ - LoggingOptions createLoggingOptions() { - if (Strings.isNullOrEmpty(credentialsFile)) { - return LoggingOptions.getDefaultInstance(); - } else { - try { - return LoggingOptions.newBuilder() - .setCredentials(GoogleCredentials.fromStream(new FileInputStream(credentialsFile))) - .build(); - } catch (IOException e) { - throw new RuntimeException( - String.format( - "Could not read credentials file %s. Please verify that the file exists and is a valid Google credentials file.", - credentialsFile), - e); + /** Gets the {@link LoggingOptions} to use for this {@link LoggingAppender}. */ + LoggingOptions getLoggingOptions() { + if (loggingOptions == null) { + if (Strings.isNullOrEmpty(credentialsFile)) { + loggingOptions = LoggingOptions.getDefaultInstance(); + } else { + try { + loggingOptions = + LoggingOptions.newBuilder() + .setCredentials( + GoogleCredentials.fromStream(new FileInputStream(credentialsFile))) + .build(); + } catch (IOException e) { + throw new RuntimeException( + String.format( + "Could not read credentials file %s. Please verify that the file exists and is a valid Google credentials file.", + credentialsFile), + e); + } } } + return loggingOptions; } private LogEntry logEntryFor(ILoggingEvent e) { diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java index 06a485d01aec..9b66d88dc501 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java @@ -32,6 +32,7 @@ import com.google.cloud.logging.LogEntry; import com.google.cloud.logging.Logging; import com.google.cloud.logging.Logging.WriteOption; +import com.google.cloud.logging.LoggingOptions; import com.google.cloud.logging.Payload.StringPayload; import com.google.cloud.logging.Severity; import com.google.common.collect.ImmutableMap; @@ -214,15 +215,28 @@ public void testMdcValuesAreConvertedToLabels() { @Test public void testCreateLoggingOptions() { + // Try to build LoggingOptions with custom credentials. final String nonExistentFile = "/path/to/non/existent/file"; LoggingAppender appender = new LoggingAppender(); appender.setCredentialsFile(nonExistentFile); try { - appender.createLoggingOptions(); + 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); + } } private LoggingEvent createLoggingEvent(Level level, long timestamp) { From 29cac4799beedb7d54b9bb8b9611a1c9ada7bc53 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 29 Mar 2019 07:15:40 +0100 Subject: [PATCH 5/5] updated readme to describe the credentialsFile property --- .../google-cloud-logging-logback/README.md | 20 +++---------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/README.md b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/README.md index 54fc6a62aa75..183a8d62a4c0 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/README.md +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-logging-logback/README.md @@ -50,6 +50,8 @@ See [Logback filters](https://logback.qos.ch/manual/filters.html#thresholdFilter INFO application.log + + /path/to/credentials.json com.example.enhancers.TestLoggingEnhancer com.example.enhancers.AnotherEnhancer WARN @@ -88,23 +90,7 @@ Authentication See the [Authentication](https://github.com/googleapis/google-cloud-java#authentication) section in the base directory's README. -You can also specify custom credentials and other options by creating a subclass of `com.google.cloud.logging.logback.LoggingAppender` and override the method `createLoggingOptions()`. Your logback.xml configuration file must reference your subclass instead of `com.google.cloud.logging.logback.LoggingAppender`. - -```java -public class CustomLoggingAppender extends LoggingAppender { - @Override - public LoggingOptions createLoggingOptions() { - try { - return LoggingOptions.newBuilder() - .setCredentials(GoogleCredentials.fromStream( - new FileInputStream("/path/to/credentials.json"))) - .build(); - } catch (IOException e) { - throw new RuntimeException("Could not find credentials", e); - } - } -} -``` +You can also specify custom credentials by setting the optional property credentialsFile in your configuration file. Limitations -----------