Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for JSON logging to stderr #1027

Merged
merged 1 commit into from
Sep 8, 2022
Merged

feat: Add support for JSON logging to stderr #1027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.logging.Filter;
import java.util.logging.Formatter;
import java.util.logging.Level;
Expand All @@ -44,6 +45,7 @@ class LoggingConfig {
private static final String USE_INHERITED_CONTEXT = "useInheritedContext";
private static final String AUTO_POPULATE_METADATA = "autoPopulateMetadata";
private static final String REDIRECT_TO_STDOUT = "redirectToStdout";
private static final String LOG_TARGET = "logTarget";

public LoggingConfig(String className) {
this.className = className;
Expand Down Expand Up @@ -87,6 +89,10 @@ Boolean getRedirectToStdout() {
return getBooleanProperty(REDIRECT_TO_STDOUT, null);
}

Optional<LoggingHandler.LogTarget> getLogTarget() {
return Optional.ofNullable(getProperty(LOG_TARGET)).map(LoggingHandler.LogTarget::valueOf);
}

MonitoredResource getMonitoredResource(String projectId) {
String resourceType = getProperty(RESOURCE_TYPE_TAG, "");
return MonitoredResourceUtil.getResource(projectId, resourceType);
Expand Down
77 changes: 64 additions & 13 deletions google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,15 @@
* Logging (defaults to {@code true}).
* <li>{@code com.google.cloud.logging.LoggingHandler.redirectToStdout} is a boolean flag that
* opts-in redirecting the output of the handler to STDOUT instead of ingesting logs to Cloud
* Logging using Logging API (defaults to {@code true}). Redirecting logs can be used in
* Logging using Logging API (defaults to {@code false}). Redirecting logs can be used in
* Google Cloud environments with installed logging agent to delegate log ingestions to the
* agent. Redirected logs are formatted as one line Json string following the structured
* logging guidelines.
* logging guidelines. This flag is deprecated; use {@code
* com.google.cloud.logging.LoggingHandler.logTarget} instead.
* <li>{@code com.google.cloud.logging.LoggingHandler.logTarget} is an enumeration controlling log
* routing (defaults to {@code CLOUD_LOGGING}). If set to STDOUT or STDERR, logs will be
* printed to the corresponding stream in the Json format that can be parsed by the logging
* agent. If set to CLOUD_LOGGING, logs will be sent directly to the Google Cloud Logging API.
* </ul>
*
* <p>To add a {@code LoggingHandler} to an existing {@link Logger} and be sure to avoid infinite
Expand All @@ -136,6 +141,16 @@ public class LoggingHandler extends Handler {
private static final String LEVEL_NAME_KEY = "levelName";
private static final String LEVEL_VALUE_KEY = "levelValue";

/** Where to send logs. */
public enum LogTarget {
/** Sends logs to the Cloud Logging API. */
CLOUD_LOGGING,
/** Sends JSON-formatted logs to stdout, for use with the Google Cloud logging agent. */
STDOUT,
/** Sends JSON-formatted logs to stderr, for use with the Google Cloud logging agent. */
STDERR
}

private final List<LoggingEnhancer> enhancers;
private final LoggingOptions loggingOptions;

Expand All @@ -152,7 +167,7 @@ public class LoggingHandler extends Handler {
private volatile Level flushLevel;

private volatile Boolean autoPopulateMetadata;
private volatile Boolean redirectToStdout;
private volatile LogTarget logTarget;

private final WriteOption[] defaultWriteOptions;

Expand Down Expand Up @@ -243,7 +258,13 @@ public LoggingHandler(
Boolean f1 = loggingOptions.getAutoPopulateMetadata();
Boolean f2 = config.getAutoPopulateMetadata();
autoPopulateMetadata = isTrueOrNull(f1) && isTrueOrNull(f2);
redirectToStdout = firstNonNull(config.getRedirectToStdout(), Boolean.FALSE);
logTarget =
config
.getLogTarget()
.orElse(
firstNonNull(config.getRedirectToStdout(), Boolean.FALSE)
? LogTarget.STDOUT
: LogTarget.CLOUD_LOGGING);
String logName = log != null ? log : config.getLogName();
MonitoredResource resource =
firstNonNull(
Expand Down Expand Up @@ -310,18 +331,24 @@ public void publish(LogRecord record) {
if (logEntry != null) {
try {
Iterable<LogEntry> logEntries =
redirectToStdout
? Instrumentation.populateInstrumentationInfo(ImmutableList.of(logEntry)).y()
: ImmutableList.of(logEntry);
logTarget == LogTarget.CLOUD_LOGGING
? ImmutableList.of(logEntry)
: Instrumentation.populateInstrumentationInfo(ImmutableList.of(logEntry)).y();
if (autoPopulateMetadata) {
logEntries =
logging.populateMetadata(
logEntries, getMonitoredResource(), "com.google.cloud.logging", "java");
}
if (redirectToStdout) {
logEntries.forEach(log -> System.out.println(log.toStructuredJsonString()));
} else {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it stands out to me that this is an if else and is replaced with a switch without a default case, but 🤷

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nickkieffer , are you using redirectToStdout feature?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, not directly anyways

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nickkieffer , I must admit that I have some confusion while following the issue you raised, but let me try answering best based on my understanding:

  1. Correct me if I wrong, you use a Python logger - based on sample code you shared. This change to print to STDERR was performed only in Java library and I believe we do not have such feature in Python. If problem indeed related to Python SDK, can you please raise the issue in Python repo? Feel free to provide all details you mentioned here and also the details about environment you run in - adding @daniel-sanche for visibility.
  2. This change supposed to use CLOUD_LOGGING by default - the STDOUT and STDERR logging needs to be explicitly enabled in code or configuration so the change should be fully backward compatible to clients which does not use the feature. If you believe this is not a case, please let us know - me and @bpcreech can take a look.

Please let me know if I understood you correctly and if I can assist you any further.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can disregard me on this, I think my ops team just so happened to turn off of logging agents around the same time as your release happened. Sorry about the distraction!

logging.write(logEntries, defaultWriteOptions);
switch (logTarget) {
case STDOUT:
logEntries.forEach(log -> System.out.println(log.toStructuredJsonString()));
break;
case STDERR:
logEntries.forEach(log -> System.err.println(log.toStructuredJsonString()));
break;
case CLOUD_LOGGING:
logging.write(logEntries, defaultWriteOptions);
break;
}
} catch (RuntimeException ex) {
getErrorManager().error(null, ex, ErrorManager.WRITE_FAILURE);
Expand Down Expand Up @@ -425,13 +452,37 @@ public Boolean getAutoPopulateMetadata() {
* Enable/disable redirection to STDOUT. If set to {@code true}, logs will be printed to STDOUT in
* the Json format that can be parsed by the logging agent. If set to {@code false}, logs will be
* ingested to Cloud Logging by calling Logging API.
*
* <p>This method is mutually exclusive with {@link #setLogTarget()}.
*
* @deprecated Use {@link #setLogTarget()}.
*/
@Deprecated
public void setRedirectToStdout(boolean value) {
this.redirectToStdout = value;
this.logTarget = value ? LogTarget.STDOUT : LogTarget.CLOUD_LOGGING;
}

/*
* @deprecated Use {@link #getLogTarget()}.
*/
@Deprecated
public Boolean getRedirectToStdout() {
return redirectToStdout;
return this.logTarget == LogTarget.STDOUT;
}

/**
* Configure the destination for ingested logs. If set to STDOUT or STDERR, logs will be printed
* to the corresponding stream in the Json format that can be parsed by the logging agent. If set
* to CLOUD_LOGGING, logs will be sent directly to the Google Cloud Logging API.
*
* <p>This method is mutually exclusive with {@link #setRedirectToStdout()}.
*/
public void setLogTarget(LogTarget value) {
this.logTarget = value;
}

public LogTarget getLogTarget() {
return logTarget;
}

/**
Expand Down
Loading