[GitHub] storm pull request #2806: [STORM-3198] Topology submitters should be able to...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2806 ---
[GitHub] storm pull request #2806: [STORM-3198] Topology submitters should be able to...
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2806#discussion_r211484225
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/supervisor/BasicContainer.java
---
@@ -622,6 +620,12 @@ protected String javaCmd(String cmd) {
List classPathParams = getClassPathParams(stormRoot,
topoVersion);
List commonParams = getCommonParams();
+String log4jConfigurationFile = getWorkerLoggingConfigFile();
--- End diff --
Thanks for explaining.
---
[GitHub] storm pull request #2806: [STORM-3198] Topology submitters should be able to...
Github user jacobtolar commented on a diff in the pull request: https://github.com/apache/storm/pull/2806#discussion_r210769170 --- Diff: pom.xml --- @@ -278,7 +278,7 @@ 4.1.25.Final 1.0.2 1.6.6 -2.8.2 +2.11.0 --- End diff -- not aware of any such reason; done ---
[GitHub] storm pull request #2806: [STORM-3198] Topology submitters should be able to...
Github user jacobtolar commented on a diff in the pull request:
https://github.com/apache/storm/pull/2806#discussion_r210768599
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/supervisor/BasicContainer.java
---
@@ -622,6 +620,12 @@ protected String javaCmd(String cmd) {
List classPathParams = getClassPathParams(stormRoot,
topoVersion);
List commonParams = getCommonParams();
+String log4jConfigurationFile = getWorkerLoggingConfigFile();
--- End diff --
No, it can't -- The xml file might be in the topology jar uploaded to the
nimbus. That file is not on the classpath if the LogWriter feature is used.
So I guess it could, but getCommonParams would have to take a parameter and
get invoked twice. Right now it's just invoked once and the output is used for
both LogWriter and Worker jvm startup scripts.
---
[GitHub] storm pull request #2806: [STORM-3198] Topology submitters should be able to...
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2806#discussion_r210681943
--- Diff:
storm-server/src/main/java/org/apache/storm/daemon/supervisor/BasicContainer.java
---
@@ -622,6 +620,12 @@ protected String javaCmd(String cmd) {
List classPathParams = getClassPathParams(stormRoot,
topoVersion);
List commonParams = getCommonParams();
+String log4jConfigurationFile = getWorkerLoggingConfigFile();
--- End diff --
Nit: Can this go in `getCommonParams`?
---
[GitHub] storm pull request #2806: [STORM-3198] Topology submitters should be able to...
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/2806#discussion_r210683327 --- Diff: pom.xml --- @@ -278,7 +278,7 @@ 4.1.25.Final 1.0.2 1.6.6 -2.8.2 +2.11.0 --- End diff -- Unless we know of a reason not to, we could just bump to the latest version IMO. ---
[GitHub] storm pull request #2806: [STORM-3198] Topology submitters should be able to...
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2806#discussion_r210682266
--- Diff: storm-client/src/jvm/org/apache/storm/Config.java ---
@@ -705,6 +705,13 @@
*/
@isString(acceptedValues = { "S0", "S1", "S2", "S3" })
public static final String TOPOLOGY_LOGGING_SENSITIVITY =
"topology.logging.sensitivity";
+/**
+ * Log file the user can use to configure Log4j2.
--- End diff --
Nit: Consider mentioning that the configuration file is applied in addition
to the regular config file, using these rules
https://logging.apache.org/log4j/2.x/manual/configuration.html#CompositeConfiguration.
---
[GitHub] storm pull request #2806: [STORM-3198] Topology submitters should be able to...
Github user jacobtolar commented on a diff in the pull request: https://github.com/apache/storm/pull/2806#discussion_r210671831 --- Diff: pom.xml --- @@ -278,7 +278,7 @@ 4.1.25.Final 1.0.2 1.6.6 -2.8.2 +2.11.0 --- End diff -- Note: The update to log4j2-2.11.0 is not strictly necessary. 2.8.2 supports composite configuration. However, there was [a bug](https://issues.apache.org/jira/browse/LOG4J2-2123) 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. ---
[GitHub] storm pull request #2806: [STORM-3198] Topology submitters should be able to...
GitHub user jacobtolar opened a pull request: https://github.com/apache/storm/pull/2806 [STORM-3198] Topology submitters should be able to supply log4j2 conf 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: ``` ``` With this child configuration added, the messages about [preparing bolts](https://github.com/apache/storm/blob/26d2f955251c0fa56520f6fe08cb35ef5171f321/storm-client/src/jvm/org/apache/storm/executor/bolt/BoltExecutor.java#L109) 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. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jacobtolar/storm STORM-3198 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2806.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2806 commit 28ad8dc77b409339768484839ab6979f42ce07ff Author: Jacob Tolar Date: 2018-08-15T19:53:42Z [STORM-3198] Topology submitters should be able to supply log4j2 configurations ---
