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/11/09 15:14:53 UTC

Review Request 27790: Alerts: Provide Summary Structure On Alerts Endpoint

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

Review request for Ambari, John Speidel, Nate Cole, and Tom Beerbower.


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


Repository: ambari


Description
-------

The web client would like to be able to request a customizable alert summary structure given a combination of parameters, such as alert definition name, host, date, etc. Currently the alerts_summary structure off of the cluster/service/host endpoints are static and just return total counts.

The new structure would also need to contain some extra information, such as original timestamp when the most recent state change occurred:

    "alerts_summary" : {
     "CRITICAL" : {
       “count”: 2,
       “original_timestamp”: 1415134996589
     },
     "OK” : {
       “count”: 45,
       “original_timestamp”: 1415134133489
     }
    ...
    }
   
The Ambari API already has a model to model in cases like this. We use a "renderer" which is a value that instructs the API engine to format the results of a query in a particular fashion. With this, we can query the alerts endpoint and format it for a summary output. This includes formatting for alerts by name, host, and other supported combinations.

Some URI examples:
http://localhost:8080/api/v1/clusters/c1/alerts?format=summary
http://localhost:8080/api/v1/clusters/c1/alerts?Alert/name=datanode_process&Alert/host_name=c6401.ambari.apache.org&format=summary


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/query/render/AlertSummaryRenderer.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertResourceDefinition.java d7aca22 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java 715d017 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertResourceProviderTest.java ef014a9 

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


Testing
-------

New tests added to ensure the renderer converts the flattened alert data correctly.


Thanks,

Jonathan Hurley


Re: Review Request 27790: Alerts: Provide Summary Structure On Alerts Endpoint

Posted by Nate Cole <nc...@hortonworks.com>.

> On Nov. 10, 2014, 9:51 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java, lines 50-63
> > <https://reviews.apache.org/r/27790/diff/1/?file=755705#file755705line50>
> >
> >     Why change to public here, I didn't see any cases outside the classes of using it
> 
> Jonathan Hurley wrote:
>     The renderer uses some of them:
>         properties.add(AlertResourceProvider.ALERT_STATE);
>         
>     I can make only the exposed ones public.

Ah, ok, didn't see that one (I only searched for ALERT_CLUSTER_NAME etc)


> On Nov. 10, 2014, 9:51 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertResourceDefinition.java, lines 45-47
> > <https://reviews.apache.org/r/27790/diff/1/?file=755704#file755704line45>
> >
> >     Don't need this doc annotation since it's implied and already using the @Override?
> 
> Jonathan Hurley wrote:
>     I love being explicit. Also, this allows us to have proper javadoc on all methods creating nice visual separation in the file.

I was dinged in my early days for unneccessary doc.


- Nate


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


On Nov. 9, 2014, 9:14 a.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27790/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2014, 9:14 a.m.)
> 
> 
> Review request for Ambari, John Speidel, Nate Cole, and Tom Beerbower.
> 
> 
> Bugs: AMBARI-8237
>     https://issues.apache.org/jira/browse/AMBARI-8237
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The web client would like to be able to request a customizable alert summary structure given a combination of parameters, such as alert definition name, host, date, etc. Currently the alerts_summary structure off of the cluster/service/host endpoints are static and just return total counts.
> 
> The new structure would also need to contain some extra information, such as original timestamp when the most recent state change occurred:
> 
>     "alerts_summary" : {
>      "CRITICAL" : {
>        “count”: 2,
>        “original_timestamp”: 1415134996589
>      },
>      "OK” : {
>        “count”: 45,
>        “original_timestamp”: 1415134133489
>      }
>     ...
>     }
>    
> The Ambari API already has a model to model in cases like this. We use a "renderer" which is a value that instructs the API engine to format the results of a query in a particular fashion. With this, we can query the alerts endpoint and format it for a summary output. This includes formatting for alerts by name, host, and other supported combinations.
> 
> Some URI examples:
> http://localhost:8080/api/v1/clusters/c1/alerts?format=summary
> http://localhost:8080/api/v1/clusters/c1/alerts?Alert/name=datanode_process&Alert/host_name=c6401.ambari.apache.org&format=summary
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/query/render/AlertSummaryRenderer.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertResourceDefinition.java d7aca22 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java 715d017 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertResourceProviderTest.java ef014a9 
> 
> Diff: https://reviews.apache.org/r/27790/diff/
> 
> 
> Testing
> -------
> 
> New tests added to ensure the renderer converts the flattened alert data correctly.
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 27790: Alerts: Provide Summary Structure On Alerts Endpoint

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

> On Nov. 10, 2014, 9:51 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java, lines 50-63
> > <https://reviews.apache.org/r/27790/diff/1/?file=755705#file755705line50>
> >
> >     Why change to public here, I didn't see any cases outside the classes of using it

The renderer uses some of them:
    properties.add(AlertResourceProvider.ALERT_STATE);
    
I can make only the exposed ones public.


> On Nov. 10, 2014, 9:51 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertResourceDefinition.java, lines 45-47
> > <https://reviews.apache.org/r/27790/diff/1/?file=755704#file755704line45>
> >
> >     Don't need this doc annotation since it's implied and already using the @Override?

I love being explicit. Also, this allows us to have proper javadoc on all methods creating nice visual separation in the file.


- Jonathan


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


On Nov. 9, 2014, 9:14 a.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27790/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2014, 9:14 a.m.)
> 
> 
> Review request for Ambari, John Speidel, Nate Cole, and Tom Beerbower.
> 
> 
> Bugs: AMBARI-8237
>     https://issues.apache.org/jira/browse/AMBARI-8237
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The web client would like to be able to request a customizable alert summary structure given a combination of parameters, such as alert definition name, host, date, etc. Currently the alerts_summary structure off of the cluster/service/host endpoints are static and just return total counts.
> 
> The new structure would also need to contain some extra information, such as original timestamp when the most recent state change occurred:
> 
>     "alerts_summary" : {
>      "CRITICAL" : {
>        “count”: 2,
>        “original_timestamp”: 1415134996589
>      },
>      "OK” : {
>        “count”: 45,
>        “original_timestamp”: 1415134133489
>      }
>     ...
>     }
>    
> The Ambari API already has a model to model in cases like this. We use a "renderer" which is a value that instructs the API engine to format the results of a query in a particular fashion. With this, we can query the alerts endpoint and format it for a summary output. This includes formatting for alerts by name, host, and other supported combinations.
> 
> Some URI examples:
> http://localhost:8080/api/v1/clusters/c1/alerts?format=summary
> http://localhost:8080/api/v1/clusters/c1/alerts?Alert/name=datanode_process&Alert/host_name=c6401.ambari.apache.org&format=summary
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/query/render/AlertSummaryRenderer.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertResourceDefinition.java d7aca22 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java 715d017 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertResourceProviderTest.java ef014a9 
> 
> Diff: https://reviews.apache.org/r/27790/diff/
> 
> 
> Testing
> -------
> 
> New tests added to ensure the renderer converts the flattened alert data correctly.
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 27790: Alerts: Provide Summary Structure On Alerts Endpoint

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

Ship it!



ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertResourceDefinition.java
<https://reviews.apache.org/r/27790/#comment101947>

    Don't need this doc annotation since it's implied and already using the @Override?



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java
<https://reviews.apache.org/r/27790/#comment101948>

    Why change to public here, I didn't see any cases outside the classes of using it


- Nate Cole


On Nov. 9, 2014, 9:14 a.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27790/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2014, 9:14 a.m.)
> 
> 
> Review request for Ambari, John Speidel, Nate Cole, and Tom Beerbower.
> 
> 
> Bugs: AMBARI-8237
>     https://issues.apache.org/jira/browse/AMBARI-8237
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The web client would like to be able to request a customizable alert summary structure given a combination of parameters, such as alert definition name, host, date, etc. Currently the alerts_summary structure off of the cluster/service/host endpoints are static and just return total counts.
> 
> The new structure would also need to contain some extra information, such as original timestamp when the most recent state change occurred:
> 
>     "alerts_summary" : {
>      "CRITICAL" : {
>        “count”: 2,
>        “original_timestamp”: 1415134996589
>      },
>      "OK” : {
>        “count”: 45,
>        “original_timestamp”: 1415134133489
>      }
>     ...
>     }
>    
> The Ambari API already has a model to model in cases like this. We use a "renderer" which is a value that instructs the API engine to format the results of a query in a particular fashion. With this, we can query the alerts endpoint and format it for a summary output. This includes formatting for alerts by name, host, and other supported combinations.
> 
> Some URI examples:
> http://localhost:8080/api/v1/clusters/c1/alerts?format=summary
> http://localhost:8080/api/v1/clusters/c1/alerts?Alert/name=datanode_process&Alert/host_name=c6401.ambari.apache.org&format=summary
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/query/render/AlertSummaryRenderer.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertResourceDefinition.java d7aca22 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java 715d017 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertResourceProviderTest.java ef014a9 
> 
> Diff: https://reviews.apache.org/r/27790/diff/
> 
> 
> Testing
> -------
> 
> New tests added to ensure the renderer converts the flattened alert data correctly.
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 27790: Alerts: Provide Summary Structure On Alerts Endpoint

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27790/#review60598
-----------------------------------------------------------

Ship it!


Looks good!

- Tom Beerbower


On Nov. 9, 2014, 2:14 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27790/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2014, 2:14 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Nate Cole, and Tom Beerbower.
> 
> 
> Bugs: AMBARI-8237
>     https://issues.apache.org/jira/browse/AMBARI-8237
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The web client would like to be able to request a customizable alert summary structure given a combination of parameters, such as alert definition name, host, date, etc. Currently the alerts_summary structure off of the cluster/service/host endpoints are static and just return total counts.
> 
> The new structure would also need to contain some extra information, such as original timestamp when the most recent state change occurred:
> 
>     "alerts_summary" : {
>      "CRITICAL" : {
>        “count”: 2,
>        “original_timestamp”: 1415134996589
>      },
>      "OK” : {
>        “count”: 45,
>        “original_timestamp”: 1415134133489
>      }
>     ...
>     }
>    
> The Ambari API already has a model to model in cases like this. We use a "renderer" which is a value that instructs the API engine to format the results of a query in a particular fashion. With this, we can query the alerts endpoint and format it for a summary output. This includes formatting for alerts by name, host, and other supported combinations.
> 
> Some URI examples:
> http://localhost:8080/api/v1/clusters/c1/alerts?format=summary
> http://localhost:8080/api/v1/clusters/c1/alerts?Alert/name=datanode_process&Alert/host_name=c6401.ambari.apache.org&format=summary
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/query/render/AlertSummaryRenderer.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertResourceDefinition.java d7aca22 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java 715d017 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertResourceProviderTest.java ef014a9 
> 
> Diff: https://reviews.apache.org/r/27790/diff/
> 
> 
> Testing
> -------
> 
> New tests added to ensure the renderer converts the flattened alert data correctly.
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>