You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Jonathan Hurley <jh...@hortonworks.com> on 2014/07/02 18:47:26 UTC

Review Request 23234: Add more info about the Heartbeat message from Ambari Agent to Server in its logs

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23234/
-----------------------------------------------------------

Review request for Ambari, Nate Cole and Sid Wagle.


Bugs: AMBARI-6356
    https://issues.apache.org/jira/browse/AMBARI-6356


Repository: ambari


Description
-------

The major request here was for some better logging of IP addresses between the server and the agents. There were some areas where, on an Exception, the python code would reference an undefined local variable, causing the exception message to be lost. Those were all fixed up.


Diffs
-----

  ambari-agent/src/main/python/ambari_agent/Controller.py 9839313 
  ambari-agent/src/main/python/ambari_agent/Heartbeat.py 6c7543e 
  ambari-agent/src/main/python/ambari_agent/LiveStatus.py 49cea62 
  ambari-agent/src/main/python/ambari_agent/NetUtil.py ece7b8f 
  ambari-agent/src/main/python/ambari_agent/main.py 78dde9e 
  ambari-agent/src/test/python/ambari_agent/TestController.py 30e03c4 
  ambari-agent/src/test/python/ambari_agent/TestMain.py afe9b59 

Diff: https://reviews.apache.org/r/23234/diff/


Testing
-------

[INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 157 licence.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 6.401s
[INFO] Finished at: Wed Jul 02 12:32:32 EDT 2014
[INFO] Final Memory: 7M/81M
[INFO] ------------------------------------------------------------------------


Thanks,

Jonathan Hurley


Re: Review Request 23234: Add more info about the Heartbeat message from Ambari Agent to Server in its logs

Posted by Jonathan Hurley <jh...@hortonworks.com>.

> On July 2, 2014, 2:48 p.m., Nate Cole wrote:
> > ambari-agent/src/main/python/ambari_agent/Controller.py, lines 93-97
> > <https://reviews.apache.org/r/23234/diff/1/?file=622789#file622789line93>
> >
> >     This may lead to some confusion on 0.0.0.0 when trying to connect on a bad name.  May want to consider moving the logger.info("Registering...") into each appropriate block (one for failure and one for success).  Or we should fail straight up if the host could not be resolved.
> >     
> >     Also, the doc says "If the host name is an IPv4 address itself it is returned unchanged" - not sure if an actual network resolution will occur then.

0.0.0.0 is typically used as a way to represent "No IP Address". I considered using a placeholder like "unknown" but then I figured that foo.server.com (unknown) looked odd compared to foo.server.com (0.0.0.0). At least with 0's you can tell we're trying to display an IP.

Network resolution won't occur if an IP is used, but I didn't want to start dealing with cases where we try to determine if an IP was used and then we don't do the lookup. 

Let me take a look at the registration log messages as you suggest.


> On July 2, 2014, 2:48 p.m., Nate Cole wrote:
> > ambari-agent/src/main/python/ambari_agent/Heartbeat.py, lines 92-94
> > <https://reviews.apache.org/r/23234/diff/1/?file=622790#file622790line92>
> >
> >     (nit) I like this pattern, but if we do this for debug, why not do it for info messages too?  Also use with debug isn't consistent either.  I don't think this patch should have to address it.

The agent's configuration file specifies that DEBUG and INFO are the valid levels. Considering this, INFO is always logged, so there's no reason to every put INFO-level statements into an if-block.

As for DEBUG, yes, I think we were initially doing concatenation of the strings here instead of parameterizations. That's why I grouped into into DEBUG. When I parameterized them, I left it in this block, but you're right, it's not consistent. I agree that there should be a JIRA that says "Make Agent Logging Consistent and Elagent".


> On July 2, 2014, 2:48 p.m., Nate Cole wrote:
> > ambari-agent/src/main/python/ambari_agent/NetUtil.py, line 52
> > <https://reviews.apache.org/r/23234/diff/1/?file=622792#file622792line52>
> >
> >     Should be debug?

In general, I didn't want to reduce scope of any log message in the event that people depended on them. In this case, it was originally INFO:
    
  logger.info("Calling url received " + str(status))

But without any valuable information. At least now we have the URL.


> On July 2, 2014, 2:48 p.m., Nate Cole wrote:
> > ambari-agent/src/main/python/ambari_agent/NetUtil.py, line 75
> > <https://reviews.apache.org/r/23234/diff/1/?file=622792#file622792line75>
> >
> >     Debug?

Same as above: the level was originally INFO, yet it was printing DEBUG in the message. I'm fine with "Trying to connection to" being INFO level as this JIRA originally was addressing network issues at a customer site.


- Jonathan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23234/#review47219
-----------------------------------------------------------


On July 2, 2014, 12:47 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23234/
> -----------------------------------------------------------
> 
> (Updated July 2, 2014, 12:47 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Sid Wagle.
> 
> 
> Bugs: AMBARI-6356
>     https://issues.apache.org/jira/browse/AMBARI-6356
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The major request here was for some better logging of IP addresses between the server and the agents. There were some areas where, on an Exception, the python code would reference an undefined local variable, causing the exception message to be lost. Those were all fixed up.
> 
> 
> Diffs
> -----
> 
>   ambari-agent/src/main/python/ambari_agent/Controller.py 9839313 
>   ambari-agent/src/main/python/ambari_agent/Heartbeat.py 6c7543e 
>   ambari-agent/src/main/python/ambari_agent/LiveStatus.py 49cea62 
>   ambari-agent/src/main/python/ambari_agent/NetUtil.py ece7b8f 
>   ambari-agent/src/main/python/ambari_agent/main.py 78dde9e 
>   ambari-agent/src/test/python/ambari_agent/TestController.py 30e03c4 
>   ambari-agent/src/test/python/ambari_agent/TestMain.py afe9b59 
> 
> Diff: https://reviews.apache.org/r/23234/diff/
> 
> 
> Testing
> -------
> 
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 157 licence.
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 6.401s
> [INFO] Finished at: Wed Jul 02 12:32:32 EDT 2014
> [INFO] Final Memory: 7M/81M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 23234: Add more info about the Heartbeat message from Ambari Agent to Server in its logs

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23234/#review47219
-----------------------------------------------------------


Yay whitespace!


ambari-agent/src/main/python/ambari_agent/Controller.py
<https://reviews.apache.org/r/23234/#comment82856>

    This may lead to some confusion on 0.0.0.0 when trying to connect on a bad name.  May want to consider moving the logger.info("Registering...") into each appropriate block (one for failure and one for success).  Or we should fail straight up if the host could not be resolved.
    
    Also, the doc says "If the host name is an IPv4 address itself it is returned unchanged" - not sure if an actual network resolution will occur then.



ambari-agent/src/main/python/ambari_agent/Heartbeat.py
<https://reviews.apache.org/r/23234/#comment82857>

    (nit) I like this pattern, but if we do this for debug, why not do it for info messages too?  Also use with debug isn't consistent either.  I don't think this patch should have to address it.



ambari-agent/src/main/python/ambari_agent/NetUtil.py
<https://reviews.apache.org/r/23234/#comment82858>

    Should be debug?



ambari-agent/src/main/python/ambari_agent/NetUtil.py
<https://reviews.apache.org/r/23234/#comment82859>

    Debug?



ambari-agent/src/main/python/ambari_agent/main.py
<https://reviews.apache.org/r/23234/#comment82860>

    See earlier comment.  Should be the same.


- Nate Cole


On July 2, 2014, 12:47 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23234/
> -----------------------------------------------------------
> 
> (Updated July 2, 2014, 12:47 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Sid Wagle.
> 
> 
> Bugs: AMBARI-6356
>     https://issues.apache.org/jira/browse/AMBARI-6356
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The major request here was for some better logging of IP addresses between the server and the agents. There were some areas where, on an Exception, the python code would reference an undefined local variable, causing the exception message to be lost. Those were all fixed up.
> 
> 
> Diffs
> -----
> 
>   ambari-agent/src/main/python/ambari_agent/Controller.py 9839313 
>   ambari-agent/src/main/python/ambari_agent/Heartbeat.py 6c7543e 
>   ambari-agent/src/main/python/ambari_agent/LiveStatus.py 49cea62 
>   ambari-agent/src/main/python/ambari_agent/NetUtil.py ece7b8f 
>   ambari-agent/src/main/python/ambari_agent/main.py 78dde9e 
>   ambari-agent/src/test/python/ambari_agent/TestController.py 30e03c4 
>   ambari-agent/src/test/python/ambari_agent/TestMain.py afe9b59 
> 
> Diff: https://reviews.apache.org/r/23234/diff/
> 
> 
> Testing
> -------
> 
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 157 licence.
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 6.401s
> [INFO] Finished at: Wed Jul 02 12:32:32 EDT 2014
> [INFO] Final Memory: 7M/81M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 23234: Add more info about the Heartbeat message from Ambari Agent to Server in its logs

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23234/#review47229
-----------------------------------------------------------

Ship it!


Ship It!

- Nate Cole


On July 2, 2014, 3:49 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23234/
> -----------------------------------------------------------
> 
> (Updated July 2, 2014, 3:49 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Sid Wagle.
> 
> 
> Bugs: AMBARI-6356
>     https://issues.apache.org/jira/browse/AMBARI-6356
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The major request here was for some better logging of IP addresses between the server and the agents. There were some areas where, on an Exception, the python code would reference an undefined local variable, causing the exception message to be lost. Those were all fixed up.
> 
> 
> Diffs
> -----
> 
>   ambari-agent/src/main/python/ambari_agent/Controller.py 9839313 
>   ambari-agent/src/main/python/ambari_agent/Heartbeat.py 6c7543e 
>   ambari-agent/src/main/python/ambari_agent/LiveStatus.py 49cea62 
>   ambari-agent/src/main/python/ambari_agent/NetUtil.py ece7b8f 
>   ambari-agent/src/main/python/ambari_agent/main.py 78dde9e 
>   ambari-agent/src/test/python/ambari_agent/TestController.py 30e03c4 
>   ambari-agent/src/test/python/ambari_agent/TestMain.py afe9b59 
> 
> Diff: https://reviews.apache.org/r/23234/diff/
> 
> 
> Testing
> -------
> 
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 157 licence.
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 6.401s
> [INFO] Finished at: Wed Jul 02 12:32:32 EDT 2014
> [INFO] Final Memory: 7M/81M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 23234: Add more info about the Heartbeat message from Ambari Agent to Server in its logs

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23234/
-----------------------------------------------------------

(Updated July 2, 2014, 3:49 p.m.)


Review request for Ambari, Nate Cole and Sid Wagle.


Bugs: AMBARI-6356
    https://issues.apache.org/jira/browse/AMBARI-6356


Repository: ambari


Description
-------

The major request here was for some better logging of IP addresses between the server and the agents. There were some areas where, on an Exception, the python code would reference an undefined local variable, causing the exception message to be lost. Those were all fixed up.


Diffs (updated)
-----

  ambari-agent/src/main/python/ambari_agent/Controller.py 9839313 
  ambari-agent/src/main/python/ambari_agent/Heartbeat.py 6c7543e 
  ambari-agent/src/main/python/ambari_agent/LiveStatus.py 49cea62 
  ambari-agent/src/main/python/ambari_agent/NetUtil.py ece7b8f 
  ambari-agent/src/main/python/ambari_agent/main.py 78dde9e 
  ambari-agent/src/test/python/ambari_agent/TestController.py 30e03c4 
  ambari-agent/src/test/python/ambari_agent/TestMain.py afe9b59 

Diff: https://reviews.apache.org/r/23234/diff/


Testing
-------

[INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 157 licence.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 6.401s
[INFO] Finished at: Wed Jul 02 12:32:32 EDT 2014
[INFO] Final Memory: 7M/81M
[INFO] ------------------------------------------------------------------------


Thanks,

Jonathan Hurley


Re: Review Request 23234: Add more info about the Heartbeat message from Ambari Agent to Server in its logs

Posted by Sid Wagle <sw...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23234/#review47216
-----------------------------------------------------------

Ship it!


Ship It!

- Sid Wagle


On July 2, 2014, 4:47 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23234/
> -----------------------------------------------------------
> 
> (Updated July 2, 2014, 4:47 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Sid Wagle.
> 
> 
> Bugs: AMBARI-6356
>     https://issues.apache.org/jira/browse/AMBARI-6356
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The major request here was for some better logging of IP addresses between the server and the agents. There were some areas where, on an Exception, the python code would reference an undefined local variable, causing the exception message to be lost. Those were all fixed up.
> 
> 
> Diffs
> -----
> 
>   ambari-agent/src/main/python/ambari_agent/Controller.py 9839313 
>   ambari-agent/src/main/python/ambari_agent/Heartbeat.py 6c7543e 
>   ambari-agent/src/main/python/ambari_agent/LiveStatus.py 49cea62 
>   ambari-agent/src/main/python/ambari_agent/NetUtil.py ece7b8f 
>   ambari-agent/src/main/python/ambari_agent/main.py 78dde9e 
>   ambari-agent/src/test/python/ambari_agent/TestController.py 30e03c4 
>   ambari-agent/src/test/python/ambari_agent/TestMain.py afe9b59 
> 
> Diff: https://reviews.apache.org/r/23234/diff/
> 
> 
> Testing
> -------
> 
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 157 licence.
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 6.401s
> [INFO] Finished at: Wed Jul 02 12:32:32 EDT 2014
> [INFO] Final Memory: 7M/81M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 23234: Add more info about the Heartbeat message from Ambari Agent to Server in its logs

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23234/#review47221
-----------------------------------------------------------

Ship it!


Ship It!

- Nate Cole


On July 2, 2014, 12:47 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23234/
> -----------------------------------------------------------
> 
> (Updated July 2, 2014, 12:47 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Sid Wagle.
> 
> 
> Bugs: AMBARI-6356
>     https://issues.apache.org/jira/browse/AMBARI-6356
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The major request here was for some better logging of IP addresses between the server and the agents. There were some areas where, on an Exception, the python code would reference an undefined local variable, causing the exception message to be lost. Those were all fixed up.
> 
> 
> Diffs
> -----
> 
>   ambari-agent/src/main/python/ambari_agent/Controller.py 9839313 
>   ambari-agent/src/main/python/ambari_agent/Heartbeat.py 6c7543e 
>   ambari-agent/src/main/python/ambari_agent/LiveStatus.py 49cea62 
>   ambari-agent/src/main/python/ambari_agent/NetUtil.py ece7b8f 
>   ambari-agent/src/main/python/ambari_agent/main.py 78dde9e 
>   ambari-agent/src/test/python/ambari_agent/TestController.py 30e03c4 
>   ambari-agent/src/test/python/ambari_agent/TestMain.py afe9b59 
> 
> Diff: https://reviews.apache.org/r/23234/diff/
> 
> 
> Testing
> -------
> 
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 157 licence.
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 6.401s
> [INFO] Finished at: Wed Jul 02 12:32:32 EDT 2014
> [INFO] Final Memory: 7M/81M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>