Skip to content
This repository has been archived by the owner on Aug 4, 2020. It is now read-only.

API v1.0: custom loggers not available on Google App Engine #63

Closed
nikolash opened this issue Jan 17, 2015 · 22 comments
Closed

API v1.0: custom loggers not available on Google App Engine #63

nikolash opened this issue Jan 17, 2015 · 22 comments

Comments

@nikolash
Copy link

Hello!

i could not use the official Java PayPal API in the past, because unfortunately the loading of the configuration properties file failed on GAE (on v0.12.2). This has been solved in your recent release v1.0.

Unfortunately, you introduced log4j. GAE does not support custom loggers; for reference please have a look here:

https://code.google.com/p/googleappengine/issues/detail?id=11499&q=log4j&colspec=ID%20Type%20Component%20Status%20Stars%20Summary%20Language%20Priority%20Owner%20Log

Could you please provide a solution to this problem that allows us to use the API in v1.0 on GAE, thus providing a method to disable log4j support?

Thank you very much!
Nikolas

@juwlee
Copy link
Contributor

juwlee commented Jan 20, 2015

Hi @nikolash are you running into any issue or is there something you would like to see from logs?
Also, I wonder if GAE complains about log4j2 when you try to call this SDK? If so, can you share the error message?

@nikolash
Copy link
Author

Hi!

thank you for contacting me.

The problem is, that you instantiate log4j directly instead of using the Java logging API as an abstraction layer. But: java.util.logging.LogManager is a restricted class on Google App Engine, which leads to an error:

Uncaught exception from servlet
java.lang.ExceptionInInitializerError
at org.apache.logging.log4j.status.StatusLogger.<clinit>(StatusLogger.java:55)
at org.apache.logging.log4j.LogManager.<clinit>(LogManager.java:56)
at com.paypal.base.rest.PayPalResource.<clinit>(PayPalResource.java:29)
at com.myapp.config.initializer.PayPalContextListener.contextInitialized(PayPalContextListener.java:19)
at org.mortbay.jetty.handler.ContextHandler.startContext(ContextHandler.java:548)
at org.mortbay.jetty.servlet.Context.startContext(Context.java:136)
at org.mortbay.jetty.webapp.WebAppContext.startContext(WebAppContext.java:1250)
at org.mortbay.jetty.handler.ContextHandler.doStart(ContextHandler.java:517)
at org.mortbay.jetty.webapp.WebAppContext.doStart(WebAppContext.java:467)
at org.mortbay.component.AbstractLifeCycle.start(AbstractLifeCycle.java:50)
at com.google.apphosting.runtime.jetty.AppVersionHandlerMap.createHandler(AppVersionHandlerMap.java:199)
at com.google.apphosting.runtime.jetty.AppVersionHandlerMap.getHandler(AppVersionHandlerMap.java:174)
at com.google.apphosting.runtime.jetty.JettyServletEngineAdapter.serviceRequest(JettyServletEngineAdapter.java:134)
at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.run(JavaRuntime.java:484)
at com.google.tracing.TraceContext$TraceContextRunnable.runInContext(TraceContext.java:438)
at com.google.tracing.TraceContext$TraceContextRunnable$1.run(TraceContext.java:445)
at com.google.tracing.CurrentContext.runInContext(CurrentContext.java:220)
at com.google.tracing.TraceContext$AbstractTraceContextCallback.runInInheritedContextNoUnref(TraceContext.java:309)
at com.google.tracing.TraceContext$AbstractTraceContextCallback.runInInheritedContext(TraceContext.java:301)
at com.google.tracing.TraceContext$TraceContextRunnable.run(TraceContext.java:442)
at com.google.apphosting.runtime.ThreadGroupPool$PoolEntry.run(ThreadGroupPool.java:251)
at java.lang.Thread.run(Thread.java:724)
Caused by: java.security.AccessControlException: access denied ("java.lang.RuntimePermission" "getClassLoader")
at java.security.AccessControlContext.checkPermission(AccessControlContext.java:375)
at java.security.AccessController.checkPermission(AccessController.java:565)
at java.lang.SecurityManager.checkPermission(SecurityManager.java:549)
at com.google.apphosting.runtime.security.CustomSecurityManager.checkPermission(CustomSecurityManager.java:56)
at java.lang.ClassLoader.checkClassLoaderPermission(ClassLoader.java:1596)
at java.lang.ClassLoader.getSystemClassLoader(ClassLoader.java:1515)
at org.apache.logging.log4j.util.LoaderUtil.findUrlResources(LoaderUtil.java:192)
at org.apache.logging.log4j.util.LoaderUtil.findResources(LoaderUtil.java:183)
at org.apache.logging.log4j.util.PropertiesUtil.<init>(PropertiesUtil.java:90)
at org.apache.logging.log4j.util.PropertiesUtil.<clinit>(PropertiesUtil.java:36)
... 22 more

The PayPalContextListener only calls the initConfig() method with a file providing the sdk settings.

I would be great if you could offer a fix to this problem.

Thanks!
Nikolas

@juwlee
Copy link
Contributor

juwlee commented Jan 20, 2015

@nikolash thanks for sharing the exception. It is thrown while initializing logger. A fix would be to initialize it selectively, meaning that the log4j classes are instantiated if environment is not GAE. Having said that, can you share the environment variables that identifies that the env is GAE?

@nikolash
Copy link
Author

I cannot confirm that, i deployed to GAE and it tries to instantiate log4j. The only setting i made was setting http.GoogleAppEngine = true. Is there anything else i have to do to tell the SDK that it runs on GAE? How can i extract the variables that you are talking about?

Thanks a lot!

On Tuesday, 20. January 2015 at 22:10, Juwon Lee wrote:

@nikolash (https://github.com/nikolash) thanks for sharing the exception. It is thrown while initializing logger. A fix would be to initialize it selectively, meaning that the log4j classes are instantiated if environment is not GAE. Having said that, can you share the environment variables that identifies that the env is GAE?


Reply to this email directly or view it on GitHub (#63 (comment)).

@nikolash
Copy link
Author

nikolash commented Feb 2, 2015

Hello Juwon,

do you have any updates on this topic for me? You labeled it as
enhancement, but for me it is a showstopper because i cannot use the SDK on
GAE. Can i expect a fix anytime soon? Otherwise i do have to replicate the
SDK api myself, which would be very unfortunate.

Thanks! Nikolas

On Tue, Jan 20, 2015 at 10:10 PM, Juwon Lee notifications@github.com
wrote:

@nikolash https://github.com/nikolash thanks for sharing the exception.
It is thrown while initializing logger. A fix would be to initialize it
selectively, meaning that the log4j classes are instantiated if environment
is not GAE. Having said that, can you share the environment variables that
identifies that the env is GAE?


Reply to this email directly or view it on GitHub
#63 (comment)
.

@jaypatel512
Copy link
Contributor

Hey Nikolash,

We would definitely get this fixed as soon as possible. however, as you could understand it may take some more time than normal issues, as we would have to setup our GAE and test it on different machines to resolve such issue.

In the mean time, if you have a specific idea/solution in mind, please feel free to send us a Pull Request, even if it is not entirely polished with everything. It would help us get started with resolving such issues faster.

Thanks.

@ghost
Copy link

ghost commented Feb 6, 2015

Hi,
I am affected as well - after spending a day on it I could not get Log4j to work in App Angine.

I solved the problem by completely getting rid of the log4j dependency - replacing it instead with java.util.logging.Logger:

I think there are only 6 classes in total where:

private static final Logger log = LogManager.getLogger(XXX.class);

has to be replaced with:

private static final Logger log = Logger.getLogger(XXX.class.getName());

plus there are a few modifications to take into account the minimal syntax differences between Log4j and the Java logger.

@nikolash
Copy link
Author

nikolash commented Feb 6, 2015

Thank you for doing this! I was planning the same. This is the proper way
to use logging; Log4J should never be initialized explicitly, that's
exactly why the Java Logging API was created.

Could you please create a pull request for the SDK-API-Team, so that your
change will make it into the next official release?

Thanks!
Nikolas

On Fri, Feb 6, 2015 at 3:11 PM, thedayofcondor notifications@github.com
wrote:

Hi,
I am affected as well - after spending a day on it I could not get Log4j
to work in App Angine.

I solved the problem by completely getting rid of the log4j dependency -
replacing it instead with java.util.logging.Logger:

I think there are only 6 classes in total where:

private static final Logger log = LogManager.getLogger(XXX.class);

has to be replaced with:

private static final Logger log = Logger.getLogger(XXX.class.getName());

plus there are a few modifications to take into account the minimal syntax
differences between Log4j and the Java logger.


Reply to this email directly or view it on GitHub
#63 (comment)
.

@ghost
Copy link

ghost commented Feb 6, 2015

Hi,
it is done

@nikolash
Copy link
Author

nikolash commented Feb 6, 2015

Thank you!

On Friday, 6. February 2015 at 18:17, Piero Filippin wrote:

Hi,
it is done


Reply to this email directly or view it on GitHub (#63 (comment)).

@bill-brown
Copy link

Hello.

I"m affected by the init issue as well. Is a maven central release going to be posted soon?

Thanks.

@juwlee
Copy link
Contributor

juwlee commented Feb 15, 2015

To fix the GAE logging support, we probably need a bit different approach that allows both java.util.logging as well as log4j because we already have developers using log4j for their logging. Just replacing log4j initialization with j.u.l will break their code. The tricky part is that loggers are declared as static final variables, which are loaded before configuration is set. So it may not be so straightforward to determine which logger to instantiate. We will keep this issue open for tracking, but in the mean time, we will point GAE users to @thedayofcondor 's repository for workaround.

@ghost
Copy link

ghost commented Feb 15, 2015

I never used it, but slf4j (http://slf4j.org/) offers an abstraction that allows to use different pluggable logging architectures.

@juwlee
Copy link
Contributor

juwlee commented Feb 15, 2015

@thedayofcondor j.u.l seems to be the only supported logger in GAE. Unfortunately, slf4j is not supported either.

@nikolash
Copy link
Author

I think java.util.logging is the only proper way to implement logging. Log4j can be abstracted through this layer. Existing code that uses log4j directly can be fixed easily.

GAE is important and should be supported by PalPal without using unofficial forks.

Thanks!
Nikolas

On 16 Feb 2015, at 00:14, Juwon Lee notifications@github.com wrote:

To fix the GAE logging support, we probably need a bit different approach that allows both java.util.logging as well as log4j because we already have developers using log4j for their logging. Just replacing log4j initialization with j.u.l will break their code. The tricky part is that loggers are declared as static final variables, which are loaded before configuration is set. So it may not be so straightforward to determine which logger to instantiate. We will keep this issue open for tracking, but in the mean time, we will point GAE users to @thedayofcondor 's repository for workaround.


Reply to this email directly or view it on GitHub.

@ghost
Copy link

ghost commented Feb 16, 2015

Hi,
I just had a go at slf4j, I built and deployed a minimal project and it works fine in app engine.

slf4j-api.jar it is not a logger itself, but is just a meta-logger (a wrapper around a real logger), then you have to put in your classpath the actual logger implementation you want to use and the corresponding slf4j jar, either:

slf4j-jdk.jar to delegate logging to j.u.l. (eg for use with app engine)

-or-

slf4j-log4j.jar (and the log4j jars) to delegate logging to log4j (for anything else - this is not going to work in appengine)

-or-
any other supported logger the user wants to use.

Code modifications are really minimal:

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

[...]

Logger logger = LoggerFactory.getLogger(Test.class);
logger.info("Hello World");

Also, slf4j jars are really minimal in size.

@gslender
Copy link

So what is the currently proposed approach - will this be fixed anytime soon or will it be quicker for me to fork and remove the offending Log4J lines?

@nikolash
Copy link
Author

I do have the same question..

On 17 Feb 2015, at 04:53, gslender notifications@github.com wrote:

So what is the currently proposed approach - will this be fixed anytime soon or will it be quicker for me to fork and remove the offending Log4J lines?


Reply to this email directly or view it on GitHub.

@gslender
Copy link

I've decided to use a fork - really poor form that PayPal would leave the API requiring a 3rd party library and then mark this issue as an enhancement with no comment or position on what or how it will be resolved.

@ghost
Copy link

ghost commented Feb 18, 2015

@gslender - I am using my own fork in production until this is resolved (it is a showstopper for anyone using gae), once this is solved it will just package the updated PayPal jar.
It is only 5 classes to be patched and I don't see any possibility of side effects, but j.u.l. and log4j APIs are annoyingly slightly different. If I have the time I will create a pull request with slf4j, which at the moment seems a very light dependency and the best compromise.

@krico
Copy link

krico commented Feb 25, 2015

@juwlee, just realised that you didn't fix the issue, but rather closed the PR.

If you want to continue to support your log4j user base, just use slf4j. If they include the slf4j-log4j.jar then it behaves (configuration, etc) as log4j. So it is "configuration by classpath".

But please, just fix it quickly. It's a tiny change and a huge problem for users of GAE.

@krico
Copy link

krico commented Feb 26, 2015

Really, now I even did it for you :-) It's really just a question of merging my PR and releasing it...

@juwlee juwlee closed this as completed in ab0b68d Mar 5, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants