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




ambari-server/src/main/java/org/apache/ambari/server/alerts/JmxServerSideAlert.java
Lines 52 (patched)
<https://reviews.apache.org/r/62744/#comment263975>

    This should be configurable via the definiion.



ambari-server/src/main/java/org/apache/ambari/server/alerts/JmxServerSideAlert.java
Lines 73 (patched)
<https://reviews.apache.org/r/62744/#comment263976>

    /jmx should probably be parameterized? Or just make as part of the URI on 
the definition.



ambari-server/src/main/java/org/apache/ambari/server/alerts/JmxServerSideAlert.java
Lines 80 (patched)
<https://reviews.apache.org/r/62744/#comment263979>

    Documentation.



ambari-server/src/main/java/org/apache/ambari/server/alerts/JmxServerSideAlert.java
Lines 83 (patched)
<https://reviews.apache.org/r/62744/#comment263977>

    It's interesting to use the MetricsRetrievalService here. This service was 
created for use with the web client since it polls every few seconds. We 
figured it's better to respond quickly with metric data that migth be a few 
seconds old.
    
    In this case, the alert only runs once every X minutes, so it's going to 
always be working with data that was retrieved X minutes ago. 
    
    Is that the desired behavior?



ambari-server/src/main/java/org/apache/ambari/server/alerts/JmxServerSideAlert.java
Lines 86 (patched)
<https://reviews.apache.org/r/62744/#comment263978>

    Documentation.



ambari-server/src/main/java/org/apache/ambari/server/state/alert/Reporting.java
Lines 211 (patched)
<https://reviews.apache.org/r/62744/#comment263980>

    Documentation.
    
    Also - maybe call this something like `buildAlert` or `getAlert` to follow 
Java convention.



ambari-server/src/main/java/org/apache/ambari/server/state/alert/Reporting.java
Lines 225-231 (patched)
<https://reviews.apache.org/r/62744/#comment263981>

    Do you need to worry about the SKIPPED state here?


- Jonathan Hurley


On Oct. 3, 2017, 7:01 a.m., Attila Magyar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62744/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2017, 7:01 a.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-22115
>     https://issues.apache.org/jira/browse/AMBARI-22115
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This is part of AMBARI-22115. We need to define alerts on JMX metrics which 
> are pulled from a cluster that is not necessarily managed by Ambari. 
> Therefore alerts can't run on ambari-agent host. I introduced a new SERVER 
> type alert called JmxServerSideAlert that can pull the jmx metrics from a 
> arbitrary url. The url comes from the alerts.json and it may contain 
> placeholders like ${hdfs-site/dfs.namenode.http-address}. This works 
> similarly than the Python class MetricAlert.
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/alerts/JmxServerSideAlert.java
>  PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/alerts/Threshold.java 
> PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/jmx/JMXMetricHolder.java
>  81d72fb 
>   ambari-server/src/main/java/org/apache/ambari/server/state/Alert.java 
> 5d2ecc6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertUri.java
>  93801d5 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/Reporting.java
>  4aeba45 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/ServerSource.java
>  c58867a 
>   
> ambari-server/src/test/java/org/apache/ambari/server/alerts/ThresholdTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/jmx/JMXMetricHolderTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/alert/AlertUriTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62744/diff/1/
> 
> 
> Testing
> -------
> 
> Used the following alerts.json to test it
> 
> {   
>   "ONEFS":{
>     "service": [
>       { 
>         "name": "onefs_namenode_cpu",
>         "label": "OneFS NameNode Host CPU Utilization",
>         "description": "This host-level alert is triggered if CPU utilization 
> of the NameNode exceeds certain warning and critical thresholds. It checks 
> the NameNode JMX Servlet for the SystemCPULoad property. The threshold values 
> are in percent.",
>         "interval": 1,
>         "help_url": 
> "https://cwiki.apache.org/confluence/display/AMBARI/Ambari+Alerts#AmbariAlerts-ambari_agent_heartbeat";,
>         "scope": "HOST",
>         "enabled": true,
>         "source": {
>           "type": "SERVER",
>           "class": "org.apache.ambari.server.alerts.JmxServerSideAlert",
>           "uri": {
>             "http": "${hdfs-site/dfs.namenode.http-address}",
>             "https": "${hdfs-site/dfs.namenode.https-address}",
>             "https_property": "${hdfs-site/dfs.http.policy}",
>             "https_property_value": "HTTPS_ONLY",
>             "connection_timeout": 5.0
>           },
>           "reporting": {
>             "ok": {
>               "text": "{1} CPU, load {0,number,percent}"
>             },
>             "warning": {
>               "text": "{1} CPU, load {0,number,percent}",
>               "value": 5
>             },
>             "critical": {
>               "text": "{1} CPU, load {0,number,percent}",
>               "value": 20
>             },
>             "units" : "%",
>             "type": "PERCENT"
>            },
>            "jmx": {
>              "property_list": [
>               "java.lang:type=OperatingSystem/SystemCpuLoad",
>               "java.lang:type=OperatingSystem/AvailableProcessors"
>              ]
>            }
>           }
>       }
>     ]
>   }
> }
>                                         1,1           Top
> 
> 
> Thanks,
> 
> Attila Magyar
> 
>

Reply via email to