Add logging controls

[richdougherty]

We've recently integrated this agent into https://github.com/playframework/playframework. It would be nice if there was a way to control how the agent logs debug information. We would like to minimise the amount of information that our users see, so that they can focus on errors.

At the moment the Util.log method prints directly to stderr (code). Instead of always logging, would it be possible to have the agent read a system property and only enable debug logging if the property is set?

[richdougherty]

I can submit a PR for this if you think it's a good idea.

[sbordet]

@richdougherty any idea on how to do this ?

The main problem is that the agent is initialized very early in the JVM bootup. At that stage no logging library is yet available.

If we shade in the agent a logging library without renaming its package, it will likely confuse applications (think 2 versions of the same library, or multiple SLF4J bindings, etc.), because it will be in the system classpath.

If we shade and relocate the package, now there are 2 different logging configurations that you need to set (the relocated and the one used by the application).

[richdougherty]

I'm actually OK with the logging output going to System.err. There's not much choice for such a low level library. What I was suggesting was that the agent's logging is switched off by default, but it can be switched on via a system property. That means the logging would be disabled for most users unless they're specifically interested in debugging the agent's behaviour.

However if you like, I can explore integration with a proper logging library. Since the agent is loaded early, we could initially log via stderr, but provide a hook for users to override the logging behaviour once a logging library is available.

E.g. in the code below the code would go to stderr initially (if a system property is set) but then a user could install their own AgentLogger into Util.agentLogger which directs messages to their favourite logging framework.

package org.mortbay.jetty.alpn.agent;

interface AgentLogger {
  void log(String message);
}

public class StdErrAgentLogger {
  private final boolean enabled =
      "true".equals(System.getProperty("org.mortbay.jetty.alpn.agent.logging");
  public void log(String message) {
    if (enabled) { System.err.println("[jetty-alpn-agent] " + message); }
}

public class Util {
  public static AgentLogger agentLogger = new StdErrAgentLogger();
  static void log(String message) {
    agentLogger.log(message);
  }
}

[sbordet]

I'm fine with a system property that controls whether to write on System.err or not like you sketched above.

I would have done it much simpler and just add the retrieval of the system property in a static block inside class Util, using Boolean.getBoolean("org.mortbay.jetty.alpn.agent.debug").

Let me know if you can prepare a PR, otherwise the change is trivial and I can make it (but you won't have authorship).

Thanks !

[richdougherty]

@sbordet, if you're happy to do the change that would be great. I think the simpler approach you propose is better too. Thanks! :)

[sbordet]

All right, since agents can take parameters, rather than using a System property you can now pass debug=true as a parameter to the agent.

[richdougherty]

Thank you. Please have some :cake:.

[sbordet]

Thanks! 😉