Initialize the default MonitoredResource from a GAE environment #1535

gregw · 2017-01-13T11:30:17Z

This PR is based on work done for GoogleCloudPlatform/jetty-runtime#81.
The creation of a MonitoredResource based on the GAE environment variables is probably general purpose enough for the conde to be included here.

gregw · 2017-01-13T11:30:35Z

@meltsufin FYI

coveralls · 2017-01-13T11:49:30Z

Coverage decreased (-0.03%) to 83.423% when pulling 646ee05 on jetty-project:flexMonitoredResource into d4d494d on GoogleCloudPlatform:master.

meltsufin · 2017-01-13T15:04:30Z

Thanks @gregw!

@garrettjonesgoogle Can you please take a look at this? We're trying to contribute the code we've developed for the TracingLogHandler back into the library, and this is just a part of it.

google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java

@@ -310,6 +327,10 @@ private LogEntry entryFor(LogRecord record) {
        .addLabel("levelName", level.getName())
        .addLabel("levelValue", String.valueOf(level.intValue()))
        .setSeverity(severityFor(level));
+    if (gaeInstanceId != null) {
+      builder.addLabel("appengine.googleapis.com/instance_name", gaeInstanceId);
+    }


gregw · 2017-01-13T22:49:46Z

@garrettjonesgoogle

in the case of gaeInstance, if the core of this PR is accepted (creating a GAE MonitoredResource), then it would be natural for the LoggingHandler to also set the gaeInstance. Also note #1532 that currently there is no need for AsyncLoggingHandler, as the base handler is now Async.

Either way I think a pluggable strategy would be good and better than enhanceLogEntry and I'd be happy to have a go at a PR for it.

I'm thinking:

interface LoggingEnhancer {
  MonitoredResource getDefaultResource();
  void enhanceLogEntry(LogEntry.Builder builder, LogRecord record);
}

Then we can add a GaeFlexLoggingEnhancer to the build so that a logging.properties file could look like:

handlers=com.google.cloud.logging.LoggingHandler
com.google.cloud.logging.LoggingHandler.enhancer=com.google.cloud.logging.GaeFlexLoggingEnhancer

garrettjonesgoogle · 2017-01-13T23:10:21Z

You are correct that the base handler is Async, but I'm not sure if I want to keep it that way. My point is a larger one though - as soon as someone creates one subclass to customize some behavior, it prevents independent behavior customization through other subclasses.

I think the scope of enhancing MonitoredResource should actually be the same as enhancing LogEntry. So I would suggest this:

interface LoggingEnhancer {
  void enhanceMonitoredResource(MonitoredResource.Builder builder);
  void enhanceLogEntry(LogEntry.Builder builder, LogRecord record);
}

The current getDefaultResource() implementation should create a builder and add the project_id label to it, then call enhanceMonitoredResource to override/supplement values.

I would also suggest a list of enhancers, in case there are independent labels to add.

It would be great for you to pick up this enhancement if you are able.

gregw · 2017-01-14T06:16:06Z

@garrettjonesgoogle main problem with the approach that you propose is that the type of the monitored resource type cannot not be enhanced, nor does it lend itself to multiple enhancers (even if it were settable).

Perhaps the resource type can be another parameter passed either to the constructor or set in the property file:

handlers=com.google.cloud.logging.LoggingHandler
com.google.cloud.logging.LoggingHandler.resourceType=gae_app
com.google.cloud.logging.LoggingHandler.enhancers=com.google.cloud.logging.GaeFlexLoggingEnhancer,org.example.SomeOtherEnhancer

I'll prepare something along those lines first thing next week.

gregw · 2017-01-16T02:31:21Z

@garrettjonesgoogle I have updated to have a Enhancer abstraction. I've added a GaeFlexLoggingEnchancer implementation, which includes the code to be able to add a traceId to the logs.

@meltsufin The GAE log can now be setup with no code, although it is a little verbose in the logging.properties file:

handlers=com.google.cloud.logging.LoggingHandler
com.google.cloud.logging.LoggingHandler.log=gaeflex.log
com.google.cloud.logging.LoggingHandler.resourceType=gae_app
com.google.cloud.logging.LoggingHandler.enhancers=com.google.cloud.logging.GaeFlexLoggingEnhancer
com.google.cloud.logging.LoggingHandler.formatter = java.util.logging.SimpleFormatter
java.util.logging.SimpleFormatter.format=%3$s: %5$s%6$s

coveralls · 2017-01-16T02:40:00Z

Coverage decreased (-0.1%) to 83.345% when pulling e7daf1f on jetty-project:flexMonitoredResource into d4d494d on GoogleCloudPlatform:master.

coveralls · 2017-01-16T02:51:52Z

Coverage decreased (-0.1%) to 83.349% when pulling e7daf1f on jetty-project:flexMonitoredResource into d4d494d on GoogleCloudPlatform:master.

google-cloud-logging/src/main/java/com/google/cloud/logging/GaeFlexLoggingEnhancer.java

+import com.google.cloud.MonitoredResource.Builder;
+
+/**
+ * A Logging {@link Enhancer} that enhances the logging for


google-cloud-logging/src/main/java/com/google/cloud/logging/GaeFlexLoggingEnhancer.java

+
+/**
+ * A Logging {@link Enhancer} that enhances the logging for
+ * GAE Flex environment.    This enhance can be configured in


google-cloud-logging/src/main/java/com/google/cloud/logging/GaeFlexLoggingEnhancer.java

+  @Override
+  public void enhanceMonitoredResource(Builder builder) {
+    gaeInstanceId = System.getenv("GAE_INSTANCE");    // Are we running on a GAE instance?
+    if (gaeInstanceId!=null) {


google-cloud-logging/src/main/java/com/google/cloud/logging/GaeFlexLoggingEnhancer.java

+
+  @Override
+  public void enhanceMonitoredResource(Builder builder) {
+    gaeInstanceId = System.getenv("GAE_INSTANCE");    // Are we running on a GAE instance?


google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java

@@ -131,9 +134,25 @@ public LoggingHandler(String log, LoggingOptions options) {
   *
   * @param log the name of the log to which log entries are written
   * @param options options for the Stackdriver Logging service
-   * @param monitoredResource the monitored resource to which log entries refer
+   * @param monitoredResource the monitored resource to which log entries refer. If null a default


google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java

+   * @param monitoredResource the monitored resource to which log entries refer. If null a default
+   * @param enhancers List of {@link Enhancer} instances.
+   * resource is created based on the project ID.  If a Google App Engine environment is detected
+   * then a more comprehensive default resource may be created.


google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java

+
+  /**
+   * A Log Enhancer.
+   * May be used to enhanced the {@link MonitoredResource} and/or the {@link LogEntry}


coveralls · 2017-01-17T07:18:04Z

Coverage decreased (-0.1%) to 83.341% when pulling a0132cf on jetty-project:flexMonitoredResource into d4d494d on GoogleCloudPlatform:master.

coveralls · 2017-01-17T10:15:37Z

Coverage decreased (-0.1%) to 83.341% when pulling 0676de0 on jetty-project:flexMonitoredResource into d4d494d on GoogleCloudPlatform:master.

coveralls · 2017-01-17T10:38:31Z

Coverage decreased (-0.1%) to 83.35% when pulling c64eb42 on jetty-project:flexMonitoredResource into d4d494d on GoogleCloudPlatform:master.

coveralls · 2017-01-17T10:47:42Z

Coverage decreased (-0.1%) to 83.35% when pulling c64eb42 on jetty-project:flexMonitoredResource into d4d494d on GoogleCloudPlatform:master.

Test googleapis/google-cloud-java#1535 and jetty-9.4.1-SNAPSHOT

google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java

-   * @param monitoredResource the monitored resource to which log entries refer
+   * @param monitoredResource the monitored resource to which log entries refer. If it is null 
+   * then a default resource is created based on the project ID.  If a Google App Engine environment is detected
+   * then a more comprehensive default resource may be created.


google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java

@@ -310,12 +354,15 @@ private LogEntry entryFor(LogRecord record) {
        .addLabel("levelName", level.getName())
        .addLabel("levelValue", String.valueOf(level.intValue()))
        .setSeverity(severityFor(level));
+
    enhanceLogEntry(builder, record);
    return builder.build();
  }

  protected void enhanceLogEntry(LogEntry.Builder builder, LogRecord record) {


coveralls · 2017-01-19T01:53:17Z

Changes Unknown when pulling 823dda7 on jetty-project:flexMonitoredResource into ** on GoogleCloudPlatform:master**.

* Update version to 0.8.1-SNAPSHOT (#1467) Also, update versions in README to 0.8.0 * Add link to Maven Central for maven-central badge. (#1468) Used to link to the image, which wasn't super useful. * fix more races in pubsub tests Previously BlockingProcessStreamReader has a terminate() method, used to tell the Reader to stop reading from the emulator process. This causes an inter-process race. If the Reader stops before reading emulator's output, the emulator process will hang as it tries to write to stdout/stderr as there's no one to read from the other side of the pipe. Since there is no way to safely stop the Reader, this commit deletes the method and its associated test. Additionally, the timeout for LocalSystemTest is increased to 3 minutes, since the emulator, somehow, consistently takes just longer than a minute to shut down. * Regenerating SPI layer (#1501) * Converting Error Reporting and Monitoring to use resource name types * Removing formatX/parseX methods from pubsub, converting usage of the same to resource name types * New methods in Logging and PubSub * Updating grpc dependency to 1.0.3 (#1504) * Release 0.8.1 (#1512) * Fix code snippet (wrong method name) in README.md Original code snippet in _"Querying data"_ section: `..while (!queryResponse.jobComplete()) {..` This results in a compile error: _"Cannot resolve method jobComplete()"_ The correct method is `jobCompleted()` * Updating version in README files. [ci skip] * Update version to 0.8.2-alpha-SNAPSHOT * Allow path in URIs passed to newFileSystem (#1470) * This makes it easier for users who start with a URI describing a full path to get a FileSystem that can work with that path, since they no longer have to needlessly remove the path from the URI. Note that Oracle's description of newFileSystem [1] puts no restriction on the passed URI. [1] https://docs.oracle.com/javase/7/docs/api/java/nio/file/FileSystems.html#newFileSystem(java.net.URI,%20java.util.Map) * Preventing logging re-entrance at FINE level (#1523) * Preventing logging re-entrance at FINE level Also: * Reducing the scope of synchronized blocks * Removing logger exclusions except for Http2FrameLogger * Add a PathMatcher for CloudStorageFileSystem (#1469) Add a test, as well. We reuse the default PathMatcher since it does a fine job of globbing files. * Set timestamp from LogRecord (#1533) * Initialize the default MonitoredResource from a GAE environment (#1535) * BigQuery: Add support to FormatOptions for AVRO #1441 Added new constant in FormatOptions, and a corresponding factory method. Updated test cases. Confirmed that AVRO does not require special treatment (like CSV does), so no additional changes are required. * Reverting changed commited by mistake before review. * BigQuery: Add support to FormatOptions for AVRO #1441 Added new constant in FormatOptions and a corresponding factory method. Updated test cases. Confirmed that AVRO does not require special treatment (like CSV does), so no additional changes are required. * use RpcFuture and remove old BundlingSettings (#1572) * use RpcFuture and remove old BundlingSettings * Release 0.8.2 Note: This version was accidentally released to Sonatype because of experimenting with deployment commands, combined with the fact that autoReleaseAfterClose is set to true. Since releases can't be taken back, we might as well own up to the release and push the code forward. * Updating version in README files. [ci skip] * Fixing javadoc error in GaeFlexLoggingEnhancer (#1582) * get tests to compile and pass

* Issue #68 added request context scope * Issue #68 added request context scope * Issue #68 added skeleton Logging Handler * Issue #68 work in progress * Issue #68 work in progress * working implementation using a threadlocal to stop looping * Issue #68 update to latest google-cloud-java LoggingHandler * Issue #68 Updated to latest gcloud API removed copied LoggingHandlers removed instanceid from monitored resource * Issue #68 Use same labels as nginx * Issue #68 Code cleanups after review * Issue #68 Removed stackdriver logging, so this just configures JUL to capture the traceid and log to stderr * renamed traceid to traceId * gcloud api not currently used * Tunnel traceId in parameters #68 * Simplified with traceId lookup in formatter #68 * Use 8.2 beta stackdriver * Improved formatting * use logger name rather than source * run jetty from the webapp working directory so logging configuration may be relative * Testing logging Test googleapis/google-cloud-java#1535 and jetty-9.4.1-SNAPSHOT * use released 9.4.1 * Issue #68 minor clean ups * Do not CD to non existant directory * improved README * fixed Cloud SDK * Released beta of google-cloud-logging * updated google-cloud API to 0.8.3-beta * added remote test to check logging * Stackdriver logging testing improved comments check for traceid * upgrade to 0.9.2 * Test for zone * upgrade to 10.0 gcloud API * enable gae module only of GAE_INSTANCE environment variable is set * improved gae.mod documentation * GCP module Rename gae module and configuration to gcp Split the jetty.commands into jetty.commands and gcp.commands moved commands file to jetty-base/config-scripts updated setup-env-ext to run the gcp.commands when image is run with a GAE_INSTANCE set * fixed warnings * turn off stackdriver logging by default. Added instructions to enable. * Updates Update to latest openjdk-runtime with setup-env.d Update to latest jetty release * Use the PLATFORM env var * Improved jetty startup Added JETTY_PROPERTIES, JETTY_MODULES_ENABLE & JETTY_MODULES_DISABLE Removed duplicate code fron 50-jetty.bash * use launcher URL * Trimmed GCP specific configuration * Added structure tests for jetty setup script Also fixed unpack bug found as a result * Support passing just args * fix merge * Fixed test workspace paths for cloud build * review feedback * working directory is the root webapp * Improve handling of various types of command line. Fixed problem with handling of command line like "ls /var" Requires duplication of test for java from openjdk-runtime docker-entrypoint.bash, which should be modified to avoid the duplication. * upgrade cgloud API version to 0.13.0-beta * use package target * tested with FINE log level * Simplify entrypoint args processing The $@ arg array is kept complete between all scripts. * remove debug * remove debug * Test that the logging dependencies are hidden from webapp classpath * update README.md with instructions to keep INFO level on io.grpc.netty.level=INFO * Updated to lasted openjdk-runtime * fixed classloader test * fixed TODO * Update to jetty 9.4.5 and gcloud 1.0.1-SNAPSHOT * upgraded to gcloud-logging 1.0.1 * turn off debug * updated README for latest 1.0.1 gcloud-logging * update classpath exclusion \o/

Initialize the default MonitoredResource from a GAE environment

646ee05

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 13, 2017

gregw mentioned this pull request Jan 13, 2017

Stackdriver logging #68 GoogleCloudPlatform/jetty-runtime#81

Merged

garrettjonesgoogle reviewed Jan 13, 2017

View reviewed changes

Logging Enhancer

4801395

Fixed codacy bot issues

e7daf1f

garrettjonesgoogle reviewed Jan 17, 2017

View reviewed changes

Fixed review points

a0132cf

fixed codacy-bot recommendation

0676de0

gregw added 3 commits January 17, 2017 21:25

Do not create default field value if non null valued passed

0aa59f0

Added unit test

e174007

fixed codacy-bot recommendation

c64eb42

gregw added a commit to GoogleCloudPlatform/jetty-runtime that referenced this pull request Jan 18, 2017

Testing logging

f255ee1

Test googleapis/google-cloud-java#1535 and jetty-9.4.1-SNAPSHOT

garrettjonesgoogle reviewed Jan 18, 2017

View reviewed changes

gregw mentioned this pull request Jan 18, 2017

Integration tests - fix intermittent failure: Unexpected method call Logging.close() #1541

Closed

gregw added 2 commits January 19, 2017 09:48

improved javadoc

c93532a

deprecate enhanceLogEntry

3185bc2

gregw added 2 commits January 19, 2017 10:37

Merge branch 'master' into flexMonitoredResource

30c0019

fixed test after merge

823dda7

garrettjonesgoogle merged commit 34c186f into googleapis:master Jan 19, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initialize the default MonitoredResource from a GAE environment #1535

Initialize the default MonitoredResource from a GAE environment #1535

gregw commented Jan 13, 2017

gregw commented Jan 13, 2017

coveralls commented Jan 13, 2017

meltsufin commented Jan 13, 2017

This comment was marked as spam.

gregw commented Jan 13, 2017

garrettjonesgoogle commented Jan 13, 2017

gregw commented Jan 14, 2017

gregw commented Jan 16, 2017

coveralls commented Jan 16, 2017

coveralls commented Jan 16, 2017

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.

coveralls commented Jan 17, 2017

coveralls commented Jan 17, 2017

coveralls commented Jan 17, 2017

coveralls commented Jan 17, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

coveralls commented Jan 19, 2017

Initialize the default MonitoredResource from a GAE environment #1535

Initialize the default MonitoredResource from a GAE environment #1535

Conversation

gregw commented Jan 13, 2017

gregw commented Jan 13, 2017

coveralls commented Jan 13, 2017

meltsufin commented Jan 13, 2017

This comment was marked as spam.

gregw commented Jan 13, 2017

garrettjonesgoogle commented Jan 13, 2017

gregw commented Jan 14, 2017

gregw commented Jan 16, 2017

coveralls commented Jan 16, 2017

coveralls commented Jan 16, 2017

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.

coveralls commented Jan 17, 2017

coveralls commented Jan 17, 2017

coveralls commented Jan 17, 2017

coveralls commented Jan 17, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

coveralls commented Jan 19, 2017