[STORM-3198] Topology submitters should be able to supply log4j2 conf

[jacobtolar]

This adds a new config setting, topology.logging.config, that allows a topology submitter to specify an additional log4j2 config to be used via composite configuration.

When the worker starts the -Dlog4j.configurationFile option is modified to include the additional file if requested.

To test, I submitted a topology (FastWordCountTopology):

bin/storm jar -c topology.logging.config=classpath:resources/topology_log4j2.xml ../../../../../examples/storm-starter/target/storm-starter-*.jar org.apache.storm.starter.FastWordCountTopology fwc

topology_log4j2.xml:

<configuration monitorInterval="60" shutdownHook="disable">
<loggers>
    <root>
       <RegexFilter regex=".*Preparing.*" onMatch="DENY" onMismatch="ACCEPT" />
    </root>
    <Logger name="org.apache.storm.metric.LoggingMetricsConsumer">
       <RegexFilter regex=".*Preparing.*" onMatch="DENY" onMismatch="ACCEPT" />
    </Logger>
    <Logger name="STDERR">
       <RegexFilter regex=".*Preparing.*" onMatch="DENY" onMismatch="ACCEPT" />
    </Logger>
    <Logger name="STDOUT">
       <RegexFilter regex=".*Preparing.*" onMatch="DENY" onMismatch="ACCEPT" />
    </Logger>
</loggers>
</configuration>

With this child configuration added, the messages about preparing bolts no longer appear in the worker log.

I also tested changing simply changing the log level in the child log4j2 config and that also works as expected.

[jacobtolar]

Note: The update to log4j2-2.11.0 is not strictly necessary. 2.8.2 supports composite configuration. However, there was a bug in merging filters from child configurations that I got fixed in 2.11.0. If a child configuration added filters and there were none in the parent, the child filter would be ignored.

I would like to add some filters in my topology, so, bumping the version to the one with the fix.

[srdo]

Unless we know of a reason not to, we could just bump to the latest version IMO.

[jacobtolar]

not aware of any such reason; done

[srdo]

Unless we know of a reason not to, we could just bump to the latest version IMO.

[jacobtolar]

Looks like there are a couple unit tests I need to fix. Probably won't get to that till next week.

[danny0405]

+1, LGTM

[revans2]

I love the change +1 for that, but @jacobtolar there are unit tests that verify exactly what is coming out on the command line, and they are failing because you changed the order of the commands on the command line.

Please take a look at org.apache.storm.daemon.supervisor.BasicContainerTest

[jacobtolar]

Yeah, sorry it took me a while to get to fixing it. Hopefully that does it.

[kishorvpatil]

👍

[srdo]

+1. Please squash to one commit @jacobtolar.

[revans2]

+1

[jacobtolar]

rebased to latest master and squashed into one commit