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

Jump to bottom

feat: add writeSynchronicity flag to appender configuration #542

Merged
merged 4 commits into from Sep 17, 2021
Merged

feat: add writeSynchronicity flag to appender configuration #542

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df573a0
feat: add support for write synchronicity config
minherz Sep 16, 2021
b898352
chore: set default value and remove debug output
minherz Sep 16, 2021
24f4bf4
chore: remove async testing from samples
minherz Sep 16, 2021
9b6e588
chore: change type of writeSynchronicity to enum
minherz Sep 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 30 additions & 3 deletions src/main/java/com/google/cloud/logging/logback/LoggingAppender.java
View file
Open in desktop
Expand Up @@ -33,6 +33,7 @@
import com.google.cloud.logging.MonitoredResourceUtil;
import com.google.cloud.logging.Payload;
import com.google.cloud.logging.Severity;
import com.google.cloud.logging.Synchronicity;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import java.io.FileInputStream;
Expand Down Expand Up @@ -63,8 +64,12 @@
* <!-- Optional: defaults to "ERROR" -->
* <flushLevel>WARNING</flushLevel>
*
* <!-- Optional: defaults to "ASYNC" -->
* <writeSynchronicity>SYNC</writeSynchronicity>
*
* &lt;!-- Optional: auto detects on App Engine Flex, Standard, GCE and GKE, defaults to "global". See <a
* href="https://cloud.google.com/logging/docs/api/v2/resource-list">supported resource types</a> --&gt;
* href=
* "https://cloud.google.com/logging/docs/api/v2/resource-list">supported resource types</a> --&gt;
* &lt;resourceType&gt;&lt;/resourceType&gt;
*
* &lt;!-- Optional: defaults to the default credentials of the environment --&gt;
Expand Down Expand Up @@ -96,6 +101,7 @@ public class LoggingAppender extends UnsynchronizedAppenderBase<ILoggingEvent> {
private String log;
private String resourceType;
private String credentialsFile;
private Synchronicity writeSyncFlag = Synchronicity.ASYNC;
private final Set<String> enhancerClassNames = new HashSet<>();
private final Set<String> loggingEventEnhancerClassNames = new HashSet<>();

Expand All @@ -122,8 +128,9 @@ public void setLog(String log) {
/**
* Sets the name of the monitored resource (Optional).
*
* <p>Must be a <a href="https://cloud.google.com/logging/docs/api/v2/resource-list">supported</a>
* resource type. gae_app, gce_instance and container are auto-detected.
* <p>Must be a <a href=
* "https://cloud.google.com/logging/docs/api/v2/resource-list">supported</a> resource type.
* gae_app, gce_instance and container are auto-detected.
*
* <p>Defaults to "global"
*
Expand All @@ -144,6 +151,19 @@ public void setCredentialsFile(String credentialsFile) {
this.credentialsFile = credentialsFile;
}

/**
* Define synchronization mode for writing log entries.
*
* @param log flag
*/
public void setWriteSynchronicity(String flag) {
Copy link

Choose a reason for hiding this comment

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

Nit: should this be enum instead of string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The parameter is a value of the enum. There is no simple way to define specific type for logback appender parameters in the configuration. The value is parsed and in a case of failure the fallback is to the default value (ASYNC).

try {
this.writeSyncFlag = Synchronicity.valueOf(flag);
} catch (IllegalArgumentException e) {
this.writeSyncFlag = Synchronicity.ASYNC; // use default value
}
}

/** Add extra labels using classes that implement {@link LoggingEnhancer}. */
public void addEnhancer(String enhancerClassName) {
this.enhancerClassNames.add(enhancerClassName);
Expand All @@ -161,6 +181,12 @@ String getLogName() {
return (log != null) ? log : "java.log";
}

public String getWriteSynchronicity() {
Copy link

Choose a reason for hiding this comment

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

Why not returning the enum?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be symetric with setWriteSynchronicity()

return (this.writeSyncFlag != null)
? this.writeSyncFlag.toString()
: Synchronicity.ASYNC.toString();
}

MonitoredResource getMonitoredResource(String projectId) {
return MonitoredResourceUtil.getResource(projectId, resourceType);
}
Expand Down Expand Up @@ -253,6 +279,7 @@ Logging getLogging() {
synchronized (this) {
if (logging == null) {
logging = getLoggingOptions().getService();
logging.setWriteSynchronicity(writeSyncFlag);
}
}
}
Expand Down