Issue #231 - Logging Rotation and Levels by joakime · Pull Request #295 · GoogleCloudPlatform/appengine-java-vm-runtime

joakime · 2016-06-30T02:37:45Z

This PR introduces a bunch of updates to the logging to enable log rotation for both the app_.json and request_.log

We can tweak the rotation numbers in a later commit, but its currently setup as ...

app-#.json is at 3 total files of 100MB each (max size)
request-#.log is at 3 total files of 10MB each (max size)

This also includes the ability to capture log4j & commons-logging & slf4j and route it to the java.util.logging setup.

request logging is also using this new logging layer (not using the older Jetty FileRollover implementation)

….util.logging + Slf4j is now present, capturing logging events from: * commons-logging * log4j * java.util.logging * slf4j-api + Slf4j is configured to use java.util.logging as its source for output, appending, handlers, and configuration + Introducing appengine-java-logging CoreLogging with ability to configure user/app logging then system logging in a consisten way.

…s SystemLogger

aozarov · 2016-07-07T20:36:47Z

appengine-java-logging/src/main/java/com/google/apphosting/logging/CoreLogging.java

+  public static void init(File appConfigFile) throws IOException {
+    // Use App (User) Configuration specified as a file parameter
+    if (appConfigFile != null && appConfigFile.exists()) {
+      debug("Loading User Config (from file): %s", appConfigFile);


Do you think the context would be enough to make "User Config" clear or should we say "User log config..." ?

Well, we could call it "App Config" if that makes it easier to understand.

…view

aozarov · 2016-07-07T23:45:32Z

appengine-java-logging/src/main/java/com/google/apphosting/logging/CoreLogging.java

+      logManager.reset(); // close & remove existing handlers, reset existing logger levels
+      logManager.readConfiguration(is);
+    } catch (SecurityException | IOException e) {
+      warning("Warning: caught exception when reading logging properties: %s", configFile


Is the "Warning" prefix needed now?

@joakime to reply

Corrected in commit 846e4d2

gregw · 2016-07-13T06:06:42Z

@joakime with @aozarov away, I will review/test these changes before we ask @ifigotin to review/merge. So once you have updated for the latest comments and merged to match the latest async-support, ping me and I'll run some tests.

gregw · 2016-08-17T22:51:36Z

@ludoch I don't think this issue helps/hinders #318, my testing shows same (bad) results for with and without this PR. This PR is simply to establish the log rotation for both logs and a bit of an implementation cleanup. #318 will required changing what we put into the logs.

ludoch · 2016-08-17T23:45:04Z

@aozarov Would you have some cycles to review this PR? Thanks.

ludoch · 2016-08-18T00:37:42Z

ack

On Wed, Aug 17, 2016 at 3:51 PM, Greg Wilkins notifications@github.com
wrote:

@ludoch https://github.com/ludoch I don't think this issue
helps/hinders #318
#318,
my testing shows same (bad) results for with and without this PR. This PR
is simply to establish the log rotation for both logs and a bit of an
implementation cleanup. #318
#318
will required changing what we put into the logs.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#295 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAE4zY_7tc6ZFOjlW53s0O8PNVOR3_2vks5qg5B4gaJpZM4JBv77
.

aslo · 2016-08-24T23:49:26Z

@aozarov Do you have a chance to take another look at this PR? Thanks!

…-java-vm-runtime into bugs/231-julroot

aozarov · 2016-08-25T22:53:14Z

appengine-java-logging/src/test/java/com/google/apphosting/logging/JsonCaptureHandler.java

+  private List<String> events;
+
+  public JsonCaptureHandler() {
+    formatter = new JsonFormatter();


Nit. Any reason for not assigning in the declaration and avoiding the need for an explicit constructor? Also, could events be final ?

aozarov · 2016-08-26T00:19:23Z

Added some comments. Didn't finish my pass. Should be done with it tomorrow.

joakime · 2016-08-26T00:45:38Z

Wow @aozarov

Thanks for the review, I'll cover these current ones tomm with a few extra commits to this PR.

aozarov · 2016-08-26T21:03:20Z

...9-compat-base/src/main/java/com/google/apphosting/vmruntime/jetty9/RequestLoggerHandler.java

+      return record.getMessage();
+    }
+  }
+


remove one empty line

aozarov · 2016-08-26T21:30:52Z

Done with my review.

…pdates

aozarov · 2016-08-30T01:33:20Z

...9-compat-base/src/main/java/com/google/apphosting/vmruntime/jetty9/RequestLoggerHandler.java

 import java.util.logging.Logger;

+/**
+ * The {@link java.util.logging.Handler} responsible for capturing the


Nit, "is responsible"

aozarov · 2016-08-31T21:13:14Z

Looks good. I think this comment should still be addressed but fine with merging it as is.

ludoch · 2016-09-01T02:29:31Z

LGTM, will merge soon.

ludoch · 2016-09-01T23:03:00Z

Changes in gcr.io/google_appengine/jetty9-compat:githubheadasync

ludoch · 2016-09-02T13:41:29Z

revert is pushed as

gcr.io/google_appengine/jetty9-compat:2016-09-02_00000

and

gcr.io/google_appengine/jetty9-compat:githubheadasync

joakime added 6 commits June 23, 2016 14:39

Issue GoogleCloudPlatform#231 - Improved testing of new CoreLogging

1c6132f

Issue GoogleCloudPlatform#231 - Document test log behavior a bit better.

a91f1c8

Typo

db8cc2e

Issue GoogleCloudPlatform#231 - Cleaning up RequestLog logging behavior

768d936

Issue GoogleCloudPlatform#231 - RequestLoggerHandler now executing it…

fdf9586

…s SystemLogger

aozarov reviewed Jul 7, 2016
View reviewed changes

Issue GoogleCloudPlatform#231 - Implemented suggested changes from re…

5f4683b

…view

aozarov reviewed Jul 7, 2016
View reviewed changes

Merge branch 'async-support' into bugs/231-julroot

143dccf

joakime added 3 commits August 25, 2016 14:20

Issue GoogleCloudPlatform#295 - Cleanup debug/warning messages & javadoc

846e4d2

Merge branch 'bugs/231-julroot' of github.com:jetty-project/appengine…

75f37b8

…-java-vm-runtime into bugs/231-julroot

Issue GoogleCloudPlatform#231 - new line at end of file

0c55fd7

aozarov reviewed Aug 25, 2016
View reviewed changes

aozarov reviewed Aug 26, 2016
View reviewed changes

Issue GoogleCloudPlatform#231 - More post-review updates to logging u…

204efa3

…pdates

aozarov reviewed Aug 30, 2016
View reviewed changes

Issue GoogleCloudPlatform#231 - More javadoc updates

b2632e2

ludoch merged commit 827670c into GoogleCloudPlatform:async-support Sep 1, 2016

meltsufin mentioned this pull request Sep 1, 2016

Logging Rotation and Levels Manual Merge GoogleCloudPlatform/jetty-runtime#2

Closed

jmcc0nn3ll mentioned this pull request Sep 2, 2016

NoClassDefFoundError & LinkageError caused by slf4j #326

Closed

ludoch mentioned this pull request Sep 2, 2016

Revert "Issue #231 - Logging Rotation and Levels" #327

Merged

Conversation

joakime commented Jun 30, 2016

Uh oh!

aozarov Jul 7, 2016

Choose a reason for hiding this comment

Uh oh!

joakime Jul 7, 2016

Choose a reason for hiding this comment

Uh oh!

aozarov Jul 7, 2016

Choose a reason for hiding this comment

Uh oh!

ludoch Aug 25, 2016

Choose a reason for hiding this comment

Uh oh!

joakime Aug 25, 2016

Choose a reason for hiding this comment

Uh oh!

gregw commented Jul 13, 2016

Uh oh!

gregw commented Aug 17, 2016

Uh oh!

ludoch commented Aug 17, 2016

Uh oh!

ludoch commented Aug 18, 2016

Uh oh!

aslo commented Aug 24, 2016

Uh oh!

aozarov Aug 25, 2016

Choose a reason for hiding this comment

Uh oh!

aozarov commented Aug 26, 2016

Uh oh!

joakime commented Aug 26, 2016

Uh oh!

aozarov Aug 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aozarov commented Aug 26, 2016

Uh oh!

aozarov Aug 30, 2016

Choose a reason for hiding this comment

Uh oh!

aozarov commented Aug 31, 2016

Uh oh!

ludoch commented Sep 1, 2016

Uh oh!

ludoch commented Sep 1, 2016

Uh oh!

ludoch commented Sep 2, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

aozarov Aug 26, 2016 •

edited

Loading