Jetty 12.0.x 12191 debug listener

[janbartel]

Closes #12191

This PR is to re-enable the Debug logging feature.

In order to do it, I had to extend the concept of the environment-specific context xml files such that we could define any number of eeX- properties files, naming xml files to be applied to the webapp being deployed.

Prior to this change, you were only permitted to define a single eeX.properties file, containing a single org.eclipse.jetty.deploy.environmentXml property pointing to either an absolute filename, or a filename relative to $jetty.base.

After this change, you may have any number of properties files prefixed by eeX-. Each one contains a property prefixed with org.eclipse.jetty.deploy.environmentXml that names a location of an absolute or $jetty.base relative file.

When there are multiple environment-specific xml files they are applied to the webapp being deployed in the ordering produced by the property names. So properties org.eclipse.jetty.deploy.environmentXml.foo and org.eclipse.jetty.deploy.environmentXml.bar properties will result in their context xml files being applied in the ordering:

1. org.eclipse.jetty.deploy.environmentXml.bar
2. org.eclipse.jetty.deploy.environmentXml.foo

Having done the above, we can now deploy the EE specific Debug logging feature. There are modules for ee8-debug, ee9-debug, ee10-debug (and will be ee11-debug in jetty-12.1).

Deployment documentation has also been updated to reflect the above.

[joakime]

@olamy huh? what does this mean from https://jenkins.webtide.net/blue/organizations/jenkins/jetty.project/detail/PR-12231/2/pipeline ?

[INFO] -------------------< org.eclipse.jetty:jetty-maven >--------------------
[INFO] Building Core :: Maven 12.0.13-SNAPSHOT                         [61/352]
[INFO]   from jetty-core/jetty-maven/pom.xml
[INFO] --------------------------------[ jar ]---------------------------------
Creating placeholder flownodes because failed loading originals.
ERROR: Cannot resume build because FlowNode 70 for FlowHead 1 could not be loaded. This is expected to happen when using the PERFORMANCE_OPTIMIZED durability setting and Jenkins is not shut down cleanly. Consider investigating to understand if Jenkins was not shut down cleanly or switching to the MAX_SURVIVABILITY durability setting which should prevent this issue in most cases.
[GitHub Checks] GitHub check (name: Jenkins, status: completed) has been published.

[gregw]

Aren't these meant to be merged into the single properties file and then that is checed properties with the prefix?

[janbartel]

I don't understand your question, if you look at lines 406 and 407 the properties are all added into the general property substitution pool:

                    try (InputStream stream = Files.newInputStream(resolvedFile))
                    {
                        tmp.load(stream);
                        //put each property into our substitution pool
                        tmp.stringPropertyNames().forEach(k -> properties.put(k, tmp.getProperty(k)));

[janbartel]

@gregw can you also comment on how the DebugHandler is used? There's one in core and one in each of ee8 and ee9. The one in core is referred to by the etc/jetty-debuglog.xml but that itself is referred to by debuglog.mod which says its a deprecated module?

[gregw]

@gregw can you also comment on how the DebugHandler is used? There's one in core and one in each of ee8 and ee9. The one in core is referred to by the etc/jetty-debuglog.xml but that itself is referred to by debuglog.mod which says its a deprecated module?

The DebugHandler exists in ee8/ee9 for backwards compatibility. All we need going forward is the Core DebugHandler. I'm not sure why the debuglog module was deprecated, but I assume it was because it does the same thing as the DebugListener. But DebugListener can only be used in a servlet context, so we would still need DebugHandler for core applications or to have a single debug log over multiple contexts. So perhaps remove the deprecation and explain a bit more about how they relate to each other.

[janbartel]

I've created PR #12254 as an alternative implementation which uses the DebugHandler class instead of ee-specific DebugListeners.

As it stands with this PR, all debug logs from all contexts append to the same log file, however there is no over-arching locking, which will lead to problems.

Ideally, each context would have its own debug log, but at the time at which debug is configured, nothing identifying about the context is known: we don't know the war file, or the context path for example. So to pursue this approach we would have to use eg the hashcode of the WebAppContext instance to make a unique filename, which is not really obvious for users.

The alternate implementation in #12254 inserts only a single DebugHandler into the handler list, so thus whilst the log will contain debug from all contexts, as there is only a single writer (with appropriate locking) there are no synchronisation issues. However, we can't log information about the context to which the inbound request will be routed, because the point at which it is intercepted, the decision on routing has not yet been made.

Yet another alternative solution that would allow us to have a debug log per context might be to configure the DebugListeners in a Configuration instead. This would require a user to configure the name of the debug log file, and any other debug log characteristics as eg context attributes. It would also mean that we would need to move the DebugListener classes out of ee10-webapp ee8/9-nested and into it's own jar so that it can optionally be included on the classpath and therefore optionally enable debug.

@gregw your thoughts?

[joakime]

Ideally, each context would have its own debug log, but at the time at which debug is configured, nothing identifying about the context is known: we don't know the war file, or the context path for example. So to pursue this approach we would have to use eg the hashcode of the WebAppContext instance to make a unique filename, which is not really obvious for users.

We are using slf4j, there's nothing preventing us from using a MDC or NDC to identify the context.
From there, a sifting appender from any slf4j implementation (like logback) could split up the logs into separate log files.

[sbordet]

We are using slf4j, there's nothing preventing us from using a MDC or NDC to identify the context.

MDC is thread local, so it won't work in general.

[janbartel]

I'm closing this PR in favour of PR #12254. The problem with this PR is that all contexts will share the same log file and it is difficult to see how to make them use a separate logfile in a way that would be meaningful to users.