[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/twill/pull/40


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[hsaputra]

Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107788346
  
--- Diff: 
twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java ---
@@ -77,7 +77,7 @@
*/
   YarnTwillController(String appName, RunId runId, ZKClient zkClient,
   final ApplicationMasterLiveNodeData amLiveNodeData, 
final YarnAppClient yarnAppClient) {
-super(runId, zkClient, Collections.emptyList());
+super(appName, runId, zkClient, amLiveNodeData.getKafkaZKConnect() != 
null, Collections.emptyList());
--- End diff --

At this point is more about style than correctness. I personally prefer to 
not make the logic of determining to enable or disable logging in 
YarnTwillController but rather on object that have current state of the union.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[hsaputra]

Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107781058
  
--- Diff: 
twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
 ---
@@ -105,4 +114,17 @@ public String getRmSchedulerAddr() {
   public Map getMaxRetries() {
 return maxRetries;
   }
+
+  /**
+   * Returns the ZK connection string for the Kafka used for log 
collections,
+   * or {@code null} if log collection is disabled.
+   */
+  @Nullable
+  public String getKafkaZKConnect() {
+if (!isLogCollectionEnabled()) {
--- End diff --

Ah sorry, that will work, I was looking at snippet of change. Yeah, should 
work


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[chtyim]

Github user chtyim commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107773444
  
--- Diff: 
twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
 ---
@@ -105,4 +114,17 @@ public String getRmSchedulerAddr() {
   public Map getMaxRetries() {
 return maxRetries;
   }
+
+  /**
+   * Returns the ZK connection string for the Kafka used for log 
collections,
+   * or {@code null} if log collection is disabled.
+   */
+  @Nullable
+  public String getKafkaZKConnect() {
+if (!isLogCollectionEnabled()) {
--- End diff --

Do you mean this javadoc and the `@Nullable` annotation not enough?

```java
/**
  * Returns the ZK connection string for the Kafka used for log collections,
  * or {@code null} if log collection is disabled.
  */
@Nullable
public String getKafkaZKConnect()
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[hsaputra]

Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107771740
  
--- Diff: 
twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
 ---
@@ -105,4 +114,17 @@ public String getRmSchedulerAddr() {
   public Map getMaxRetries() {
 return maxRetries;
   }
+
+  /**
+   * Returns the ZK connection string for the Kafka used for log 
collections,
+   * or {@code null} if log collection is disabled.
+   */
+  @Nullable
+  public String getKafkaZKConnect() {
+if (!isLogCollectionEnabled()) {
--- End diff --

Same thing of no way that caller could have guessed without looking at the 
code that there is a check of isLogCollectionEnabled inside getKafkaZKConnect 
that have inferred meaning when it is returning null.

The compromise I think at least added JavaDoc header for getKafkaZKConnect 
to explain why it could return null.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[hsaputra]

Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107739693
  
--- Diff: 
twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
 ---
@@ -105,4 +114,17 @@ public String getRmSchedulerAddr() {
   public Map getMaxRetries() {
 return maxRetries;
   }
+
+  /**
+   * Returns the ZK connection string for the Kafka used for log 
collections,
+   * or {@code null} if log collection is disabled.
+   */
+  @Nullable
+  public String getKafkaZKConnect() {
+if (!isLogCollectionEnabled()) {
--- End diff --

The call to `getZkConnectStr` and `getTwillAppName` and `getTwillAppRunId` 
will never return null based on the contract (no @Nullable) , hence call to 
`getKafkaZKConnect` will never be null. 

I was saying that either way, someone need to check whether call to 
getKafkaZKConnect will return null if log collection is disabled, so why not 
ask to isLogCollectionEnabled to check first.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[chtyim]

Github user chtyim commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107717653
  
--- Diff: 
twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
 ---
@@ -105,4 +114,17 @@ public String getRmSchedulerAddr() {
   public Map getMaxRetries() {
 return maxRetries;
   }
+
+  /**
+   * Returns the ZK connection string for the Kafka used for log 
collections,
+   * or {@code null} if log collection is disabled.
+   */
+  @Nullable
+  public String getKafkaZKConnect() {
+if (!isLogCollectionEnabled()) {
--- End diff --

How to avoid returning null? You mean by throwing exception so that the 
caller has to call isLogCollectionEnabled before calling this method? I am not 
a friend of method coupling like this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[chtyim]

Github user chtyim commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107717203
  
--- Diff: 
twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java ---
@@ -77,7 +77,7 @@
*/
   YarnTwillController(String appName, RunId runId, ZKClient zkClient,
   final ApplicationMasterLiveNodeData amLiveNodeData, 
final YarnAppClient yarnAppClient) {
-super(runId, zkClient, Collections.emptyList());
+super(appName, runId, zkClient, amLiveNodeData.getKafkaZKConnect() != 
null, Collections.emptyList());
--- End diff --

The contract of the getKafkaZKConnect won't change though, since it has to 
return something (or throw exception). From the caller point of view, there is 
almost no differences.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[hsaputra]

Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107690293
  
--- Diff: 
twill-core/src/main/java/org/apache/twill/internal/AbstractTwillController.java 
---
@@ -71,21 +71,37 @@
   private static final Logger LOG = 
LoggerFactory.getLogger(AbstractTwillController.class);
   private static final Gson GSON = new Gson();
 
+  private final String appName;
+  private final RunId runId;
   private final Queue logHandlers;
   private final KafkaClientService kafkaClient;
   private ZKDiscoveryService discoveryServiceClient;
   private Cancellable logCancellable;
 
-  public AbstractTwillController(RunId runId, ZKClient zkClient, 
Iterable logHandlers) {
+  public AbstractTwillController(String appName, RunId runId, ZKClient 
zkClient, boolean logCollectionEnabled,
+ Iterable logHandlers) {
 super(runId, zkClient);
+this.appName = appName;
+this.runId = runId;
 this.logHandlers = new ConcurrentLinkedQueue<>();
-this.kafkaClient = new 
ZKKafkaClientService(ZKClients.namespace(zkClient, "/" + runId.getId() + 
"/kafka"));
-Iterables.addAll(this.logHandlers, logHandlers);
+
+// When addressing TWILL-147, need to check if the given ZKClient is
+// actually used by the Kafka used for log collection
+if (logCollectionEnabled) {
+  this.kafkaClient = new 
ZKKafkaClientService(ZKClients.namespace(zkClient, "/" + runId.getId() + 
"/kafka"));
+  Iterables.addAll(this.logHandlers, logHandlers);
+} else {
+  this.kafkaClient = null;
+  if (!Iterables.isEmpty(logHandlers)) {
+LOG.warn("Log collection is disabled for application {} with runId 
{}. " +
+   "Adding log handler won't get any logs.", appName, 
runId);
+  }
+}
   }
 
   @Override
   protected synchronized void doStartUp() {
-if (!logHandlers.isEmpty()) {
+if (kafkaClient != null && !logHandlers.isEmpty()) {
--- End diff --

You are right here, there  is a chance that even log is enabled but caller 
can pass empty `logHandlers`.
+1 for current check


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[hsaputra]

Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107689017
  
--- Diff: 
twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java ---
@@ -77,7 +77,7 @@
*/
   YarnTwillController(String appName, RunId runId, ZKClient zkClient,
   final ApplicationMasterLiveNodeData amLiveNodeData, 
final YarnAppClient yarnAppClient) {
-super(runId, zkClient, Collections.emptyList());
+super(appName, runId, zkClient, amLiveNodeData.getKafkaZKConnect() != 
null, Collections.emptyList());
--- End diff --

Yeah, thinking about having 
`ApplicationMasterLiveNode.isLogCollectionEnabled` to make the contract more 
clear. Otherwise later we will forget why checking for getKafkaZKConnect != 
null here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[chtyim]

Github user chtyim commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107347828
  
--- Diff: 
twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java ---
@@ -77,7 +77,7 @@
*/
   YarnTwillController(String appName, RunId runId, ZKClient zkClient,
   final ApplicationMasterLiveNodeData amLiveNodeData, 
final YarnAppClient yarnAppClient) {
-super(runId, zkClient, Collections.emptyList());
+super(appName, runId, zkClient, amLiveNodeData.getKafkaZKConnect() != 
null, Collections.emptyList());
--- End diff --

Or are you suggesting adding a new method `boolean isLogCollectionEnabled` 
to the `ApplicationMasterLiveNode` class?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[chtyim]

Github user chtyim commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107347590
  
--- Diff: 
twill-core/src/main/java/org/apache/twill/internal/TwillRuntimeSpecification.java
 ---
@@ -105,4 +114,17 @@ public String getRmSchedulerAddr() {
   public Map getMaxRetries() {
 return maxRetries;
   }
+
+  /**
+   * Returns the ZK connection string for the Kafka used for log 
collections,
+   * or {@code null} if log collection is disabled.
+   */
+  @Nullable
+  public String getKafkaZKConnect() {
+if (!isLogCollectionEnabled()) {
--- End diff --

What do mean by side effect check? The contract of this method is to return 
`null` if log collection is disabled.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[hsaputra]

Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107347185
  
--- Diff: 
twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillController.java ---
@@ -77,7 +77,7 @@
*/
   YarnTwillController(String appName, RunId runId, ZKClient zkClient,
   final ApplicationMasterLiveNodeData amLiveNodeData, 
final YarnAppClient yarnAppClient) {
-super(runId, zkClient, Collections.emptyList());
+super(appName, runId, zkClient, amLiveNodeData.getKafkaZKConnect() != 
null, Collections.emptyList());
--- End diff --

Can the constructor of `YarnTwillController` take new input of 
`logCollectionEnabled` ? I am seeing different ways to check whether to disable 
log or not


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[hsaputra]

Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107346167
  
--- Diff: 
twill-core/src/main/java/org/apache/twill/internal/AbstractTwillController.java 
---
@@ -71,21 +71,37 @@
   private static final Logger LOG = 
LoggerFactory.getLogger(AbstractTwillController.class);
   private static final Gson GSON = new Gson();
 
+  private final String appName;
+  private final RunId runId;
   private final Queue logHandlers;
   private final KafkaClientService kafkaClient;
   private ZKDiscoveryService discoveryServiceClient;
   private Cancellable logCancellable;
 
-  public AbstractTwillController(RunId runId, ZKClient zkClient, 
Iterable logHandlers) {
+  public AbstractTwillController(String appName, RunId runId, ZKClient 
zkClient, boolean logCollectionEnabled,
+ Iterable logHandlers) {
 super(runId, zkClient);
+this.appName = appName;
+this.runId = runId;
 this.logHandlers = new ConcurrentLinkedQueue<>();
-this.kafkaClient = new 
ZKKafkaClientService(ZKClients.namespace(zkClient, "/" + runId.getId() + 
"/kafka"));
-Iterables.addAll(this.logHandlers, logHandlers);
+
+// When addressing TWILL-147, need to check if the given ZKClient is
+// actually used by the Kafka used for log collection
+if (logCollectionEnabled) {
+  this.kafkaClient = new 
ZKKafkaClientService(ZKClients.namespace(zkClient, "/" + runId.getId() + 
"/kafka"));
+  Iterables.addAll(this.logHandlers, logHandlers);
+} else {
+  this.kafkaClient = null;
+  if (!Iterables.isEmpty(logHandlers)) {
+LOG.warn("Log collection is disabled for application {} with runId 
{}. " +
+   "Adding log handler won't get any logs.", appName, 
runId);
+  }
+}
   }
 
   @Override
   protected synchronized void doStartUp() {
-if (!logHandlers.isEmpty()) {
+if (kafkaClient != null && !logHandlers.isEmpty()) {
--- End diff --

But `this.logHandlers` will always be empty if `if (kafkaClient == null)` 
is true? So could we just rely on check on kafkaClient as null ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[chtyim]

Github user chtyim commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107344859
  
--- Diff: 
twill-core/src/main/java/org/apache/twill/internal/AbstractTwillController.java 
---
@@ -71,21 +71,37 @@
   private static final Logger LOG = 
LoggerFactory.getLogger(AbstractTwillController.class);
   private static final Gson GSON = new Gson();
 
+  private final String appName;
+  private final RunId runId;
   private final Queue logHandlers;
   private final KafkaClientService kafkaClient;
   private ZKDiscoveryService discoveryServiceClient;
   private Cancellable logCancellable;
 
-  public AbstractTwillController(RunId runId, ZKClient zkClient, 
Iterable logHandlers) {
+  public AbstractTwillController(String appName, RunId runId, ZKClient 
zkClient, boolean logCollectionEnabled,
+ Iterable logHandlers) {
 super(runId, zkClient);
+this.appName = appName;
+this.runId = runId;
 this.logHandlers = new ConcurrentLinkedQueue<>();
-this.kafkaClient = new 
ZKKafkaClientService(ZKClients.namespace(zkClient, "/" + runId.getId() + 
"/kafka"));
-Iterables.addAll(this.logHandlers, logHandlers);
+
+// When addressing TWILL-147, need to check if the given ZKClient is
+// actually used by the Kafka used for log collection
+if (logCollectionEnabled) {
+  this.kafkaClient = new 
ZKKafkaClientService(ZKClients.namespace(zkClient, "/" + runId.getId() + 
"/kafka"));
+  Iterables.addAll(this.logHandlers, logHandlers);
+} else {
+  this.kafkaClient = null;
+  if (!Iterables.isEmpty(logHandlers)) {
+LOG.warn("Log collection is disabled for application {} with runId 
{}. " +
+   "Adding log handler won't get any logs.", appName, 
runId);
+  }
+}
   }
 
   @Override
   protected synchronized void doStartUp() {
-if (!logHandlers.isEmpty()) {
+if (kafkaClient != null && !logHandlers.isEmpty()) {
--- End diff --

`logCollectionEnabled` is not a field. I used `kafkaClient == null` to 
indicate disabling of log collection.
Also, the `logHandlers` collection can be empty during startup of the 
controller, even log collection is enabled, hence the check.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[hsaputra]

Github user hsaputra commented on a diff in the pull request:

https://github.com/apache/twill/pull/40#discussion_r107344378
  
--- Diff: 
twill-core/src/main/java/org/apache/twill/internal/AbstractTwillController.java 
---
@@ -71,21 +71,37 @@
   private static final Logger LOG = 
LoggerFactory.getLogger(AbstractTwillController.class);
   private static final Gson GSON = new Gson();
 
+  private final String appName;
+  private final RunId runId;
   private final Queue logHandlers;
   private final KafkaClientService kafkaClient;
   private ZKDiscoveryService discoveryServiceClient;
   private Cancellable logCancellable;
 
-  public AbstractTwillController(RunId runId, ZKClient zkClient, 
Iterable logHandlers) {
+  public AbstractTwillController(String appName, RunId runId, ZKClient 
zkClient, boolean logCollectionEnabled,
+ Iterable logHandlers) {
 super(runId, zkClient);
+this.appName = appName;
+this.runId = runId;
 this.logHandlers = new ConcurrentLinkedQueue<>();
-this.kafkaClient = new 
ZKKafkaClientService(ZKClients.namespace(zkClient, "/" + runId.getId() + 
"/kafka"));
-Iterables.addAll(this.logHandlers, logHandlers);
+
+// When addressing TWILL-147, need to check if the given ZKClient is
+// actually used by the Kafka used for log collection
+if (logCollectionEnabled) {
+  this.kafkaClient = new 
ZKKafkaClientService(ZKClients.namespace(zkClient, "/" + runId.getId() + 
"/kafka"));
+  Iterables.addAll(this.logHandlers, logHandlers);
+} else {
+  this.kafkaClient = null;
+  if (!Iterables.isEmpty(logHandlers)) {
+LOG.warn("Log collection is disabled for application {} with runId 
{}. " +
+   "Adding log handler won't get any logs.", appName, 
runId);
+  }
+}
   }
 
   @Override
   protected synchronized void doStartUp() {
-if (!logHandlers.isEmpty()) {
+if (kafkaClient != null && !logHandlers.isEmpty()) {
--- End diff --

Should this just check `if (logCollectionEnabled)` instead since 
technically both conditions will be met when logCollectionEnabled is true


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] twill pull request #40: (TWILL-122) Allow disabling log collection

[chtyim]

GitHub user chtyim opened a pull request:

https://github.com/apache/twill/pull/40

(TWILL-122) Allow disabling log collection

- Introduced a new configuration twill.log.collection.enabled for
  turning off log collection
- Refactor YarnTwillController and related class hierarchy to not
  starting Kafka client when log collection is disabled
- Added Kafka zk connection string information in AM live node data
- Refactor KafkaAppender and ServiceMain configureLogger
  - Log to StatusManager instead of Logger to avoid recursive logging
  - Instead of resetting logback configuration, directly instantiate and
add the Kafka log appender to the logging context.
- Refactor ServiceMain, ApplicationMasterMain and TwillContainerMain to
  simplify ZK Connection string construction

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/chtyim/twill feature/TWILL-122

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/twill/pull/40.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 #40


commit 617280ee72b2e7fc7a3cc473d6dfe35576cf5e57
Author: Terence Yim 
Date:   2017-03-20T21:42:46Z

(TWILL-122) Allow disabling log collection

- Introduced a new configuration twill.log.collection.enabled for
  turning off log collection
- Refactor YarnTwillController and related class hierarchy to not
  starting Kafka client when log collection is disabled
- Added Kafka zk connection string information in AM live node data
- Refactor KafkaAppender and ServiceMain configureLogger
  - Log to StatusManager instead of Logger to avoid recursive logging
  - Instead of resetting logback configuration, directly instantiate and
add the Kafka log appender to the logging context.
- Refactor ServiceMain, ApplicationMasterMain and TwillContainerMain to
  simplify ZK Connection string construction




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---