Skip to content
This repository has been archived by the owner on Dec 12, 2024. It is now read-only.

Issue #4 Revert to default jetty logging #19

Merged
merged 4 commits into from
Sep 15, 2016

Conversation

gregw
Copy link
Contributor

@gregw gregw commented Sep 14, 2016

Reverted to default jetty logging. Tested with async-rest example, which is currently running at https://jetty9-work.appspot.com/ and the logs produced can be seen on the console.

@gregw
Copy link
Contributor Author

gregw commented Sep 14, 2016

@meltsufin
Copy link
Member

What will happen to Jetty logging if the resources/jetty-logging.properties is not there at all?

@joakime
Copy link
Contributor

joakime commented Sep 14, 2016

It would be discovered.

  1. Slf4jLog - used if slf4j-api is present on server side
  2. StdErrLog - used if slf4j-api is not present
  3. JavaUtilLog - never used

It would also make configuring it strange for people that want to change the behavior.
("where do I enable log4j? where can I set the logging levels? how can i see the stacktraces?" etc)

@joakime
Copy link
Contributor

joakime commented Sep 14, 2016

Would recommend leaving resources/jetty-logging.properties in place.

It's going to be required in the long run anyway.

Once any of the following becomes true:

  1. We use the Google Cloud Logging API
  2. We want to use java.util.logging
  3. We want to support logging.properties on the application deployment

This PR should be considered "a short term hack".

@meltsufin
Copy link
Member

My question was more about the lines that are staying in jetty-logging.properites. Are they changing any defaults?

org.eclipse.jetty.LEVEL=INFO
org.eclipse.jetty.SOURCE=false
org.eclipse.jetty.STACKS=true
org.eclipse.jetty.LONG=false

@gregw
Copy link
Contributor Author

gregw commented Sep 14, 2016

@meltsufin those are the defaults, so yes we can just delete the file, as it is essentially documentation. Any change to the file needs to replace the file in a subsequent docker image, and I have an example of doing that in the sample app

I'll remove

@meltsufin
Copy link
Member

LGTM

@gregw gregw merged commit 46e13ff into GoogleCloudPlatform:master Sep 15, 2016
@gregw gregw deleted the master-4 branch September 15, 2016 22:17
@meltsufin
Copy link
Member

@gregw I noticed that java-util-logging.properties is still there. Was it supposed to be removed as part of this PR?

@gregw
Copy link
Contributor Author

gregw commented Sep 27, 2016

@meltsufin yes that file is no longer used. Originally it was used if the jetty-logging.properties configured JUL logging, but as we have removed jetty-logging.properties we should also remove java-util-logging.properties

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