Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

[WIP] Add support for logging based on google-cloud-logging #347

Open
wants to merge 2 commits into
base: async-support
Choose a base branch
from

Conversation

meltsufin
Copy link
Member

I'm trying to port over the new logging mechanism to the compat runtime to be used with a custom logging.properties file, but not having much luck.

In my test app, I have this in webapp\WEB-INF\logging.properties:

# Set the default logging level for all loggers to INFO
.level=INFO

# Override root level
com.foo.level=FINE

# Override parent's level
com.foo.bar.level=SEVERE

handlers=com.google.cloud.logging.LoggingHandler
com.google.cloud.logging.LoggingHandler.level=FINE
com.google.cloud.logging.LoggingHandler.log=gae_app.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

and appengine-web.xml has this:

<appengine-web-app xmlns="http://appengine.google.com/ns/1.0">
  <application>YOUR-PROJECT-ID</application>
  <version>YOUR-VERSION-ID</version>
  <threadsafe>true</threadsafe>
  <env>flex</env>
  <beta-settings>
    <setting name="machine_type" value="n1-standard-1"/>
    <setting name="enable_app_engine_apis" value="true"/>
  </beta-settings>
  <system-properties>
    <property name="java.util.logging.config.file" value="WEB-INF/logging.properties"/>
  </system-properties>
</appengine-web-app>

@gregw This configuration doesn't seem to be working. I only see logs in the app stream and no sign of gae_app.log in the Stackdriver Logging UI. Any thoughts?

@gregw gregw self-assigned this Feb 1, 2017
@@ -31,6 +31,7 @@
<properties>
<maven.build.timestamp.format>yyyyMMddHHmm</maven.build.timestamp.format>
<appengine.api.version>1.9.40</appengine.api.version>
Copy link

Choose a reason for hiding this comment

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

This is probably no longer used, but if it is, it should be at least 1.9.48 if not .49

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, thanks!

@@ -31,6 +31,7 @@
<properties>
<maven.build.timestamp.format>yyyyMMddHHmm</maven.build.timestamp.format>
<appengine.api.version>1.9.40</appengine.api.version>
<gcloud.api.version>0.8.2-beta-SNAPSHOT</gcloud.api.version>
Copy link

Choose a reason for hiding this comment

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

Why are you using the -SNAPSHOT ??

You do need to initialize Cloud Logging and turn on the JUL hook to use it. -- I don't see that here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Like Greg mentioned, we don't have a non-snapshot release of it yet.

@gregw
Copy link
Contributor

gregw commented Feb 2, 2017

@meltsufin I'm not going to have a chance to run this myself for a few hours. But I'm guessing the problem is that this compat runs with the working directory at $JETTY_BASE rather than in the webapp itself. So your relative path to WEB-INF/logging.properties is not working. Try putting an absolute path there to confirm/deny

@lesv the SNAPSHOT is used because the LoggingHandler from google-cloud-java with the modifications to be enhanced for the monitored resource and traceid has not yet been released.

@meltsufin
Copy link
Member Author

@gregw It looks like WEB-INF/logging.properties is being picked up fine based of the debugging logs that I added. I think the issue has to do with the initialization of the logging handler. It's probably failing, but we don't see any logs for it anywhere.

@gregw
Copy link
Contributor

gregw commented Feb 7, 2017

@meltsufin perhaps we need a PR on google-cloud-logging that will print any exceptions to stderr so we one can better debug logging problems in init?

@meltsufin
Copy link
Member Author

@gregw Yes, that's a good idea. I also think we need to pull the zone stuff into this PR for now because it sounds like they won't be able to merge that PR and release in a while.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants