Re: Review Request 45873: Create a new alert type that is based on timeseries metrics

2016-04-11 Thread Dmytro Sen 


> On Апрель 8, 2016, 12:18 п.п., Jonathan Hurley wrote:
> > ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py, lines 66-70
> > 
> >
> > Since this is contacting AMS, is there a better place to get this 
> > information from, like the AMS configurations?
> 
> Sid Wagle wrote:
> +1 Ambari server already has the cached location and port for 
> collector(s), is there way to make that more of a first class citizen?
> 
> Dmytro Sen wrote:
> I'll fix this.

After reviewing  and debugging the code, I see that there is no a better place 
to get AMS info from.

Ambari server java code and metrics monitor python code cache the AMS collector 
info, but not Ambari agent. I believe, there is no need to implement ams info 
delivery mechanism from ambari server cache. Using configs for getting such 
info is robust, it's been used by metric alert for few releases, it also 
correctly handles the case of changing ams collector port and moving collector 
component between nodes.

Please ship the updated patch if you agree to keep the current implementation


- Dmytro


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


On Апрель 11, 2016, 3:18 п.п., Dmytro Sen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45873/
> ---
> 
> (Updated Апрель 11, 2016, 3:18 п.п.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jonathan Hurley, and Sid Wagle.
> 
> 
> Bugs: AMBARI-15766
> https://issues.apache.org/jira/browse/AMBARI-15766
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> There are a few requirements around creating alerts based on the time-series 
> data stored in AMS. They fall in the the general category of:
> - identify a metrics (name of the metrics, app name)
> - identify a window (e.g. last 10 minutes, last 1 day)
> - get the data points
> - add them up, or
> - compute average, or
> - compute standard deviations, or
> - ...
> - compare the value to a threshold
> - emit an alert if needed (WARN, CRITICAL, etc)
> 
> We should also allow for alerts based on multiple metrics but there is no ask 
> for such a capability. Just something to keep in mind while designing.
> It seems general enough that we should define an alert type that can be 
> instantiated by services to define specific alerts. Users can add additional 
> alerts if they so choose.
> 
> 
> Diffs
> -
> 
>   ambari-agent/src/main/python/ambari_agent/AlertSchedulerHandler.py b84832d 
>   ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py PRE-CREATION 
>   ambari-agent/src/main/python/ambari_agent/alerts/metric_alert.py d177bd4 
>   ambari-agent/src/test/python/ambari_agent/TestAlertSchedulerHandler.py 
> f4e7ba1 
>   ambari-agent/src/test/python/ambari_agent/TestAlerts.py 9caee8a 
>   ambari-agent/src/test/python/ambari_agent/TestAmsAlert.py PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/aggregate_functions.py 
> PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/firewall.py 64d396b 
>   ambari-common/src/main/python/ambari_commons/urllib_handlers.py aa2e3f6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java
>  1676b53 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AmsSource.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/MetricSource.java
>  2ff438d 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/SourceType.java
>  357baf9 
>   ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/alerts.json 
> 3612de2 
>   
> ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/alerts/alert_metrics_deviation.py
>  038592f 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/services/AmbariMetaInfoTest.java
>  5dc7f3b 
> 
> Diff: https://reviews.apache.org/r/45873/diff/
> 
> 
> Testing
> ---
> 
> Unit tests passed
> 
> 
> Thanks,
> 
> Dmytro Sen
> 
>

Re: Review Request 45873: Create a new alert type that is based on timeseries metrics

2016-04-11 Thread Dmytro Sen 


> On Апрель 8, 2016, 12:18 п.п., Jonathan Hurley wrote:
> > ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py, lines 117-118
> > 
> >
> > Any reason you chose to use httplib here instead of urllib2? I think 
> > that urllib2 handles more for you automatically, like redirects and stuff.
> > 
> > If there's a good reason or you don't think urllib2 is a good fit, then 
> > you can disregard this.

That's a common pattern in ambari metrics monitor and service check python code 
to use httplib.HTTPConnection , just want to keep all the code similar.

+ we already have tcp-connection reusing and HTTPS support implementation with 
httplib.HTTPConnection 
- no need for redirects and stuff :)


- Dmytro


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


On Апрель 7, 2016, 3:24 п.п., Dmytro Sen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45873/
> ---
> 
> (Updated Апрель 7, 2016, 3:24 п.п.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jonathan Hurley, and Sid Wagle.
> 
> 
> Bugs: AMBARI-15766
> https://issues.apache.org/jira/browse/AMBARI-15766
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> There are a few requirements around creating alerts based on the time-series 
> data stored in AMS. They fall in the the general category of:
> - identify a metrics (name of the metrics, app name)
> - identify a window (e.g. last 10 minutes, last 1 day)
> - get the data points
> - add them up, or
> - compute average, or
> - compute standard deviations, or
> - ...
> - compare the value to a threshold
> - emit an alert if needed (WARN, CRITICAL, etc)
> 
> We should also allow for alerts based on multiple metrics but there is no ask 
> for such a capability. Just something to keep in mind while designing.
> It seems general enough that we should define an alert type that can be 
> instantiated by services to define specific alerts. Users can add additional 
> alerts if they so choose.
> 
> 
> Diffs
> -
> 
>   ambari-agent/src/main/python/ambari_agent/AlertSchedulerHandler.py b84832d 
>   ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py PRE-CREATION 
>   ambari-agent/src/main/python/ambari_agent/alerts/metric_alert.py d177bd4 
>   ambari-agent/src/test/python/ambari_agent/TestAlertSchedulerHandler.py 
> f4e7ba1 
>   ambari-agent/src/test/python/ambari_agent/TestAlerts.py e5f6a41 
>   ambari-agent/src/test/python/ambari_agent/TestAmsAlert.py PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/aggregation_functions.py 
> PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/firewall.py 64d396b 
>   ambari-common/src/main/python/ambari_commons/urllib_handlers.py aa2e3f6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java
>  1676b53 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AmsSource.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/MetricSource.java
>  2ff438d 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/SourceType.java
>  357baf9 
>   ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/alerts.json 
> 3612de2 
>   
> ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/alerts/alert_metrics_deviation.py
>  038592f 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/services/AmbariMetaInfoTest.java
>  10b92d4 
> 
> Diff: https://reviews.apache.org/r/45873/diff/
> 
> 
> Testing
> ---
> 
> Unit tests passed
> 
> 
> Thanks,
> 
> Dmytro Sen
> 
>

Re: Review Request 45873: Create a new alert type that is based on timeseries metrics

2016-04-11 Thread Dmytro Sen 


> On Апрель 8, 2016, 12:18 п.п., Jonathan Hurley wrote:
> > ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py, line 24
> > 
> >
> > Why not urllib2?

urllib2 doesn't have urlencode() method.

Common usage urllib2 to making GET with url-encoded parameters is
data = {'var1': 'val1'}
data = urllib.urlencode(data)
urllib2.urlopen(foo_url, data)


- Dmytro


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


On Апрель 7, 2016, 3:24 п.п., Dmytro Sen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45873/
> ---
> 
> (Updated Апрель 7, 2016, 3:24 п.п.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jonathan Hurley, and Sid Wagle.
> 
> 
> Bugs: AMBARI-15766
> https://issues.apache.org/jira/browse/AMBARI-15766
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> There are a few requirements around creating alerts based on the time-series 
> data stored in AMS. They fall in the the general category of:
> - identify a metrics (name of the metrics, app name)
> - identify a window (e.g. last 10 minutes, last 1 day)
> - get the data points
> - add them up, or
> - compute average, or
> - compute standard deviations, or
> - ...
> - compare the value to a threshold
> - emit an alert if needed (WARN, CRITICAL, etc)
> 
> We should also allow for alerts based on multiple metrics but there is no ask 
> for such a capability. Just something to keep in mind while designing.
> It seems general enough that we should define an alert type that can be 
> instantiated by services to define specific alerts. Users can add additional 
> alerts if they so choose.
> 
> 
> Diffs
> -
> 
>   ambari-agent/src/main/python/ambari_agent/AlertSchedulerHandler.py b84832d 
>   ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py PRE-CREATION 
>   ambari-agent/src/main/python/ambari_agent/alerts/metric_alert.py d177bd4 
>   ambari-agent/src/test/python/ambari_agent/TestAlertSchedulerHandler.py 
> f4e7ba1 
>   ambari-agent/src/test/python/ambari_agent/TestAlerts.py e5f6a41 
>   ambari-agent/src/test/python/ambari_agent/TestAmsAlert.py PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/aggregation_functions.py 
> PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/firewall.py 64d396b 
>   ambari-common/src/main/python/ambari_commons/urllib_handlers.py aa2e3f6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java
>  1676b53 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AmsSource.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/MetricSource.java
>  2ff438d 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/SourceType.java
>  357baf9 
>   ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/alerts.json 
> 3612de2 
>   
> ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/alerts/alert_metrics_deviation.py
>  038592f 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/services/AmbariMetaInfoTest.java
>  10b92d4 
> 
> Diff: https://reviews.apache.org/r/45873/diff/
> 
> 
> Testing
> ---
> 
> Unit tests passed
> 
> 
> Thanks,
> 
> Dmytro Sen
> 
>

Re: Review Request 45873: Create a new alert type that is based on timeseries metrics

2016-04-11 Thread Dmytro Sen 


> On Апрель 7, 2016, 11:57 п.п., Sid Wagle wrote:
> > ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py, line 157
> > 
> >
> > OrderedDict support is only 2.7+

Made it unordered. For currently supported aggregation function metrics order 
isn't required


- Dmytro


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


On Апрель 7, 2016, 3:24 п.п., Dmytro Sen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45873/
> ---
> 
> (Updated Апрель 7, 2016, 3:24 п.п.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jonathan Hurley, and Sid Wagle.
> 
> 
> Bugs: AMBARI-15766
> https://issues.apache.org/jira/browse/AMBARI-15766
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> There are a few requirements around creating alerts based on the time-series 
> data stored in AMS. They fall in the the general category of:
> - identify a metrics (name of the metrics, app name)
> - identify a window (e.g. last 10 minutes, last 1 day)
> - get the data points
> - add them up, or
> - compute average, or
> - compute standard deviations, or
> - ...
> - compare the value to a threshold
> - emit an alert if needed (WARN, CRITICAL, etc)
> 
> We should also allow for alerts based on multiple metrics but there is no ask 
> for such a capability. Just something to keep in mind while designing.
> It seems general enough that we should define an alert type that can be 
> instantiated by services to define specific alerts. Users can add additional 
> alerts if they so choose.
> 
> 
> Diffs
> -
> 
>   ambari-agent/src/main/python/ambari_agent/AlertSchedulerHandler.py b84832d 
>   ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py PRE-CREATION 
>   ambari-agent/src/main/python/ambari_agent/alerts/metric_alert.py d177bd4 
>   ambari-agent/src/test/python/ambari_agent/TestAlertSchedulerHandler.py 
> f4e7ba1 
>   ambari-agent/src/test/python/ambari_agent/TestAlerts.py e5f6a41 
>   ambari-agent/src/test/python/ambari_agent/TestAmsAlert.py PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/aggregation_functions.py 
> PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/firewall.py 64d396b 
>   ambari-common/src/main/python/ambari_commons/urllib_handlers.py aa2e3f6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java
>  1676b53 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AmsSource.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/MetricSource.java
>  2ff438d 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/SourceType.java
>  357baf9 
>   ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/alerts.json 
> 3612de2 
>   
> ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/alerts/alert_metrics_deviation.py
>  038592f 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/services/AmbariMetaInfoTest.java
>  10b92d4 
> 
> Diff: https://reviews.apache.org/r/45873/diff/
> 
> 
> Testing
> ---
> 
> Unit tests passed
> 
> 
> Thanks,
> 
> Dmytro Sen
> 
>

Re: Review Request 45873: Create a new alert type that is based on timeseries metrics

2016-04-11 Thread Dmytro Sen 


> On Апрель 7, 2016, 11:57 п.п., Sid Wagle wrote:
> > Very nice pythonic changes to reduce verbosity (y)
> > 
> > Why are we not chanigng the current script type alerts to use new scheme ?

current script alert is to HDFS awared, but AmsAlert is a general 
implementation for checking any component metrics.


- Dmytro


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


On Апрель 7, 2016, 3:24 п.п., Dmytro Sen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45873/
> ---
> 
> (Updated Апрель 7, 2016, 3:24 п.п.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jonathan Hurley, and Sid Wagle.
> 
> 
> Bugs: AMBARI-15766
> https://issues.apache.org/jira/browse/AMBARI-15766
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> There are a few requirements around creating alerts based on the time-series 
> data stored in AMS. They fall in the the general category of:
> - identify a metrics (name of the metrics, app name)
> - identify a window (e.g. last 10 minutes, last 1 day)
> - get the data points
> - add them up, or
> - compute average, or
> - compute standard deviations, or
> - ...
> - compare the value to a threshold
> - emit an alert if needed (WARN, CRITICAL, etc)
> 
> We should also allow for alerts based on multiple metrics but there is no ask 
> for such a capability. Just something to keep in mind while designing.
> It seems general enough that we should define an alert type that can be 
> instantiated by services to define specific alerts. Users can add additional 
> alerts if they so choose.
> 
> 
> Diffs
> -
> 
>   ambari-agent/src/main/python/ambari_agent/AlertSchedulerHandler.py b84832d 
>   ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py PRE-CREATION 
>   ambari-agent/src/main/python/ambari_agent/alerts/metric_alert.py d177bd4 
>   ambari-agent/src/test/python/ambari_agent/TestAlertSchedulerHandler.py 
> f4e7ba1 
>   ambari-agent/src/test/python/ambari_agent/TestAlerts.py e5f6a41 
>   ambari-agent/src/test/python/ambari_agent/TestAmsAlert.py PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/aggregation_functions.py 
> PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/firewall.py 64d396b 
>   ambari-common/src/main/python/ambari_commons/urllib_handlers.py aa2e3f6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java
>  1676b53 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AmsSource.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/MetricSource.java
>  2ff438d 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/SourceType.java
>  357baf9 
>   ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/alerts.json 
> 3612de2 
>   
> ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/alerts/alert_metrics_deviation.py
>  038592f 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/services/AmbariMetaInfoTest.java
>  10b92d4 
> 
> Diff: https://reviews.apache.org/r/45873/diff/
> 
> 
> Testing
> ---
> 
> Unit tests passed
> 
> 
> Thanks,
> 
> Dmytro Sen
> 
>

Re: Review Request 45873: Create a new alert type that is based on timeseries metrics

2016-04-08 Thread Jonathan Hurley 

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


Fix it, then Ship it!





ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py (line 24)


Why not urllib2?



ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py (lines 66 - 70)


Since this is contacting AMS, is there a better place to get this 
information from, like the AMS configurations?



ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py (lines 117 - 118)


Any reason you chose to use httplib here instead of urllib2? I think that 
urllib2 handles more for you automatically, like redirects and stuff.

If there's a good reason or you don't think urllib2 is a good fit, then you 
can disregard this.


- Jonathan Hurley


On April 7, 2016, 11:24 a.m., Dmytro Sen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45873/
> ---
> 
> (Updated April 7, 2016, 11:24 a.m.)
> 
> 
> Review request for Ambari, Aravindan Vijayan, Jonathan Hurley, and Sid Wagle.
> 
> 
> Bugs: AMBARI-15766
> https://issues.apache.org/jira/browse/AMBARI-15766
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> There are a few requirements around creating alerts based on the time-series 
> data stored in AMS. They fall in the the general category of:
> - identify a metrics (name of the metrics, app name)
> - identify a window (e.g. last 10 minutes, last 1 day)
> - get the data points
> - add them up, or
> - compute average, or
> - compute standard deviations, or
> - ...
> - compare the value to a threshold
> - emit an alert if needed (WARN, CRITICAL, etc)
> 
> We should also allow for alerts based on multiple metrics but there is no ask 
> for such a capability. Just something to keep in mind while designing.
> It seems general enough that we should define an alert type that can be 
> instantiated by services to define specific alerts. Users can add additional 
> alerts if they so choose.
> 
> 
> Diffs
> -
> 
>   ambari-agent/src/main/python/ambari_agent/AlertSchedulerHandler.py b84832d 
>   ambari-agent/src/main/python/ambari_agent/alerts/ams_alert.py PRE-CREATION 
>   ambari-agent/src/main/python/ambari_agent/alerts/metric_alert.py d177bd4 
>   ambari-agent/src/test/python/ambari_agent/TestAlertSchedulerHandler.py 
> f4e7ba1 
>   ambari-agent/src/test/python/ambari_agent/TestAlerts.py e5f6a41 
>   ambari-agent/src/test/python/ambari_agent/TestAmsAlert.py PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/aggregation_functions.py 
> PRE-CREATION 
>   ambari-common/src/main/python/ambari_commons/firewall.py 64d396b 
>   ambari-common/src/main/python/ambari_commons/urllib_handlers.py aa2e3f6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java
>  1676b53 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AmsSource.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/MetricSource.java
>  2ff438d 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/SourceType.java
>  357baf9 
>   ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/alerts.json 
> 3612de2 
>   
> ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/package/alerts/alert_metrics_deviation.py
>  038592f 
>   
> ambari-server/src/test/java/org/apache/ambari/server/api/services/AmbariMetaInfoTest.java
>  10b92d4 
> 
> Diff: https://reviews.apache.org/r/45873/diff/
> 
> 
> Testing
> ---
> 
> Unit tests passed
> 
> 
> Thanks,
> 
> Dmytro Sen
> 
>