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.

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

Add async/sync setting to logging handler #1716

Merged
merged 7 commits into from
Mar 9, 2017

Conversation

michaelbausor
Copy link
Contributor

Two questions:

  • I don't like WriteLogMethod much as a setting name. WriteSyncType? Others?
  • I am using a switch in write(). Any preference to instead set a callable at construction time? Or something else?

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 8, 2017
@michaelbausor
Copy link
Contributor Author

Fixes #1532

LoggingOptions options = logging().getOptions();
MonitoredResource resource = MonitoredResource.of("gce_instance",
ImmutableMap.of("project_id", options.getProjectId(),
"instance_id", "instance",
"zone", "us-central1-a"));
LoggingHandler handler = new AsyncLoggingHandler(logName, options, resource);
LoggingHandler handler = new LoggingHandler(logName, options, resource);

This comment was marked as spam.

This comment was marked as spam.

new ByteArrayInputStream(
"com.google.cloud.logging.LoggingHandler.writeLogMethod=SYNC".getBytes()));
} catch (Exception e) {
throw new RuntimeException(e);

This comment was marked as spam.

This comment was marked as spam.


package com.google.cloud.logging;

public enum WriteLogMethod {

This comment was marked as spam.

This comment was marked as spam.

@michaelbausor
Copy link
Contributor Author

PTAL. Also, should we have tests to make sure that settings in the properties file are read correctly?

@garrettjonesgoogle
Copy link
Member

Yes, I think we should have a test for the properties file settings.

@@ -296,6 +300,16 @@ Formatter getFormatterProperty(String name, Formatter defaultValue) {
}
return Collections.emptyList();
}

Synchronicity getSynchronicityProperty(String name, Synchronicity defaultValue) {

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.


package com.google.cloud.logging;

public enum Synchronicity {

This comment was marked as spam.

This comment was marked as spam.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 81.492% when pulling e7e4fc2 on michaelbausor:async-setting into 33d824d on GoogleCloudPlatform:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 81.498% when pulling 7ba4868 on michaelbausor:async-setting into 33d824d on GoogleCloudPlatform:master.

@@ -35,6 +33,7 @@
import java.util.logging.LogRecord;
import java.util.logging.Logger;
import java.util.logging.SimpleFormatter;
import com.google.cloud.logging.Logging.WriteOption;

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.


package com.google.cloud.logging;

public enum Synchronicity {

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@michaelbausor
Copy link
Contributor Author

PTAL

@garrettjonesgoogle
Copy link
Member

LGTM

@michaelbausor michaelbausor merged commit b698b13 into googleapis:master Mar 9, 2017
@michaelbausor michaelbausor deleted the async-setting branch March 9, 2017 17:17
@michaelbausor michaelbausor changed the title WIP: Add async/sync setting to logging handler Add async/sync setting to logging handler Mar 9, 2017
michaelbausor added a commit that referenced this pull request Mar 9, 2017
* Add async/sync setting to logging handler
@michaelbausor michaelbausor mentioned this pull request Mar 9, 2017
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants