You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Aravindan Vijayan <av...@hortonworks.com> on 2017/01/06 21:01:17 UTC

Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

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

Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.


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


Repository: ambari


Description
-------

Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
Counter, Timer and Avg time (Timer/Counter) metrics are tracked.

The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.


Diffs
-----

  ambari-server/conf/unix/metrics.properties 3ee22d6 
  ambari-server/conf/windows/metrics.properties 3ee22d6 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
  ambari-server/src/test/resources/metrics.properties 5eee064 

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


Testing
-------

Manually tested.
Unit tests added.


File Attachments
----------------

Sample Grafana dashboard for AmbariServer DB metrics
  https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png


Thanks,

Aravindan Vijayan


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

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

> On Jan. 6, 2017, 7:12 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java, line 109
> > <https://reviews.apache.org/r/55278/diff/2/?file=1599025#file1599025line109>
> >
> >     How do the internal workings of metricsSource.publish(...) work? If it's synchronous in any way, this could cause a massive performance issue. 
> >     
> >     If it simply queues data to be pushed to Ambari Metrics and returns immediately, then I think we're OK and you can drop the issue. 
> >     
> >     I'm just wanting to make sure we're not sneaking in a major bottleneck.
> 
> Aravindan Vijayan wrote:
>     Jonathan, it is entirely asynchronous. Even pre-processing before sending to AMS is done asynchronously.

Great - I hadn't made the connection that the source below (which enqueues runnables) is what makes this async.


> On Jan. 6, 2017, 7:12 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java, lines 106-114
> > <https://reviews.apache.org/r/55278/diff/2/?file=1599025#file1599025line106>
> >
> >     This kind of seems like a workflow problem, no? We shouldn't need to keep track of whether it was initialized.
> 
> Aravindan Vijayan wrote:
>     The AmbariPerformanceMonitor is designed to work with the DatabaseMetricsSource to send metrics to AMS. Since the AmbariPerformanceMonitor is instantiated through EclipseLink, and DatabaseMetricsSource is instantiated explicitly through MetricsService initialization, the Monitor might come up before the Source. Hence, I added this initialization check & retry. 
>     
>     One can still use the inbuilt PeformanceMonitor to look at metrics through logs if not interested in visualizing them through AMS.

Hmmm - that kind of stinks, but I see what you mean. However, when you call init() you're missing metrics, right? Since you never re-submit it.


- Jonathan


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


On Jan. 6, 2017, 5:44 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2017, 5:44 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

Posted by Aravindan Vijayan <av...@hortonworks.com>.

> On Jan. 7, 2017, 12:12 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java, lines 74-75
> > <https://reviews.apache.org/r/55278/diff/2/?file=1599025#file1599025line74>
> >
> >     You're not calling super here. We're making them choose between AmbariPerformanceMonitor and the regular PerformanceMonitor? That's probably OK - just wanted to point out and make sure the behavior is desired.

Yes. That is what I had in mind. The related ambari.properties value can be PerformanceMonitor (or) AmbariPerformanceMonitor. Let me know what you think.


- Aravindan


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


On Jan. 6, 2017, 10:44 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2017, 10:44 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

Posted by Aravindan Vijayan <av...@hortonworks.com>.

> On Jan. 7, 2017, 12:12 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java, line 109
> > <https://reviews.apache.org/r/55278/diff/2/?file=1599025#file1599025line109>
> >
> >     How do the internal workings of metricsSource.publish(...) work? If it's synchronous in any way, this could cause a massive performance issue. 
> >     
> >     If it simply queues data to be pushed to Ambari Metrics and returns immediately, then I think we're OK and you can drop the issue. 
> >     
> >     I'm just wanting to make sure we're not sneaking in a major bottleneck.

Jonathan, it is entirely asynchronous. Even pre-processing before sending to AMS is done asynchronously.


> On Jan. 7, 2017, 12:12 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java, lines 106-114
> > <https://reviews.apache.org/r/55278/diff/2/?file=1599025#file1599025line106>
> >
> >     This kind of seems like a workflow problem, no? We shouldn't need to keep track of whether it was initialized.

The AmbariPerformanceMonitor is designed to work with the DatabaseMetricsSource to send metrics to AMS. Since the AmbariPerformanceMonitor is instantiated through EclipseLink, and DatabaseMetricsSource is instantiated explicitly through MetricsService initialization, the Monitor might come up before the Source. Hence, I added this initialization check & retry. 

One can still use the inbuilt PeformanceMonitor to look at metrics through logs if not interested in visualizing them through AMS.


> On Jan. 7, 2017, 12:12 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java, lines 81-85
> > <https://reviews.apache.org/r/55278/diff/2/?file=1599026#file1599026line81>
> >
> >     Should this be configurable? Is 1 thread always enough? Anyway - can we also name this threadpool so we know what it is when looking at dumps.

I will name this threadpool. IMO, 1 thread is enough since we have a maxmimum of 1 minute (monitor collection interval) to send metrics to AMS, which should be sufficient.


- Aravindan


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


On Jan. 6, 2017, 10:44 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2017, 10:44 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

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




ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java (lines 74 - 75)
<https://reviews.apache.org/r/55278/#comment232006>

    You're not calling super here. We're making them choose between AmbariPerformanceMonitor and the regular PerformanceMonitor? That's probably OK - just wanted to point out and make sure the behavior is desired.



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java (lines 106 - 114)
<https://reviews.apache.org/r/55278/#comment232004>

    This kind of seems like a workflow problem, no? We shouldn't need to keep track of whether it was initialized.



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java (line 109)
<https://reviews.apache.org/r/55278/#comment232007>

    How do the internal workings of metricsSource.publish(...) work? If it's synchronous in any way, this could cause a massive performance issue. 
    
    If it simply queues data to be pushed to Ambari Metrics and returns immediately, then I think we're OK and you can drop the issue. 
    
    I'm just wanting to make sure we're not sneaking in a major bottleneck.



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java (line 117)
<https://reviews.apache.org/r/55278/#comment232005>

    Doc.



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java (line 65)
<https://reviews.apache.org/r/55278/#comment232010>

    Doc.



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java (lines 81 - 85)
<https://reviews.apache.org/r/55278/#comment232009>

    Should this be configurable? Is 1 thread always enough? Anyway - can we also name this threadpool so we know what it is when looking at dumps.



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java (line 87)
<https://reviews.apache.org/r/55278/#comment232008>

    Doc.



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java (line 89)
<https://reviews.apache.org/r/55278/#comment232012>

    For readability, can we make this a named inner class which implements Runnable?



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java (lines 175 - 177)
<https://reviews.apache.org/r/55278/#comment232011>

    Doc.


- Jonathan Hurley


On Jan. 6, 2017, 5:44 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2017, 5:44 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

Posted by Aravindan Vijayan <av...@hortonworks.com>.

> On Jan. 9, 2017, 6:43 p.m., Jonathan Hurley wrote:
> > ambari-server/conf/windows/metrics.properties, lines 45-46
> > <https://reviews.apache.org/r/55278/diff/2-3/?file=1599020#file1599020line45>
> >
> >     Any reason some entities are excluded by default here? Things like Alert(.*)Entity and other heavy hitters (StackEntity, StageEntity, etc).
> 
> Aravindan Vijayan wrote:
>     I just went with some initial entries for the monitored entities, to serve as a starting point. We can refine and add more based on need later.
> 
> Jonathan Hurley wrote:
>     The ones I mentioned are pretty heavy (alerts, stages, stacks) ... those metrics would be really helpful...

I will add them.


- Aravindan


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


On Jan. 9, 2017, 6:20 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2017, 6:20 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

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

> On Jan. 9, 2017, 1:43 p.m., Jonathan Hurley wrote:
> > ambari-server/conf/windows/metrics.properties, lines 45-46
> > <https://reviews.apache.org/r/55278/diff/2-3/?file=1599020#file1599020line45>
> >
> >     Any reason some entities are excluded by default here? Things like Alert(.*)Entity and other heavy hitters (StackEntity, StageEntity, etc).
> 
> Aravindan Vijayan wrote:
>     I just went with some initial entries for the monitored entities, to serve as a starting point. We can refine and add more based on need later.

The ones I mentioned are pretty heavy (alerts, stages, stacks) ... those metrics would be really helpful...


- Jonathan


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


On Jan. 9, 2017, 1:20 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2017, 1:20 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

Posted by Aravindan Vijayan <av...@hortonworks.com>.

> On Jan. 9, 2017, 6:43 p.m., Jonathan Hurley wrote:
> > ambari-server/conf/windows/metrics.properties, lines 45-46
> > <https://reviews.apache.org/r/55278/diff/2-3/?file=1599020#file1599020line45>
> >
> >     Any reason some entities are excluded by default here? Things like Alert(.*)Entity and other heavy hitters (StackEntity, StageEntity, etc).

I just went with some initial entries for the monitored entities, to serve as a starting point. We can refine and add more based on need later.


- Aravindan


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


On Jan. 9, 2017, 6:20 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2017, 6:20 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

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


Ship it!




Just one more question about the metrics configured by default ...


ambari-server/conf/windows/metrics.properties (lines 45 - 46)
<https://reviews.apache.org/r/55278/#comment232160>

    Any reason some entities are excluded by default here? Things like Alert(.*)Entity and other heavy hitters (StackEntity, StageEntity, etc).


- Jonathan Hurley


On Jan. 9, 2017, 1:20 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2017, 1:20 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

Posted by Aravindan Vijayan <av...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55278/
-----------------------------------------------------------

(Updated Jan. 9, 2017, 6:20 p.m.)


Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.


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


Repository: ambari


Description
-------

Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
Counter, Timer and Avg time (Timer/Counter) metrics are tracked.

The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.


Diffs (updated)
-----

  ambari-server/conf/unix/metrics.properties 3ee22d6 
  ambari-server/conf/windows/metrics.properties 3ee22d6 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
  ambari-server/src/test/resources/metrics.properties 5eee064 

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


Testing
-------

Manually tested.
Unit tests added.


File Attachments
----------------

Sample Grafana dashboard for AmbariServer DB metrics
  https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png


Thanks,

Aravindan Vijayan


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

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


Ship it!




Ship It!

- Sid Wagle


On Jan. 6, 2017, 10:44 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2017, 10:44 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

Posted by Aravindan Vijayan <av...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55278/
-----------------------------------------------------------

(Updated Jan. 6, 2017, 10:44 p.m.)


Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.


Changes
-------

Addressed review issues.


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


Repository: ambari


Description
-------

Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
Counter, Timer and Avg time (Timer/Counter) metrics are tracked.

The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.


Diffs (updated)
-----

  ambari-server/conf/unix/metrics.properties 3ee22d6 
  ambari-server/conf/windows/metrics.properties 3ee22d6 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
  ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
  ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
  ambari-server/src/test/resources/metrics.properties 5eee064 

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


Testing
-------

Manually tested.
Unit tests added.


File Attachments
----------------

Sample Grafana dashboard for AmbariServer DB metrics
  https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png


Thanks,

Aravindan Vijayan


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

Posted by Sid Wagle <sw...@hortonworks.com>.

> On Jan. 6, 2017, 9:23 p.m., Sid Wagle wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java, line 124
> > <https://reviews.apache.org/r/55278/diff/1/?file=1598881#file1598881line124>
> >
> >     1. Does this need to be static?
> >     2. Why doesn't it publish an interface instead of abstract impl?
> 
> Aravindan Vijayan wrote:
>     1. Does this need to be static?
>     The AmbariPerformanceMonitor is instantiated and managed through EclipseLink. For it to reach the DatabaseMetricsSource, I thought that this would be a reasonable method to do that.
>     
>     2. Why doesn't it publish an interface instead of abstract impl?
>     Agreed. Will make the change.

Fine with 1, to be left asis.


- Sid


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


On Jan. 6, 2017, 9:01 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2017, 9:01 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

Posted by Aravindan Vijayan <av...@hortonworks.com>.

> On Jan. 6, 2017, 9:23 p.m., Sid Wagle wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java, line 75
> > <https://reviews.apache.org/r/55278/diff/1/?file=1598877#file1598877line75>
> >
> >     Why not drop this, the vars are unambiguous anyways.

Done.


> On Jan. 6, 2017, 9:23 p.m., Sid Wagle wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java, line 109
> > <https://reviews.apache.org/r/55278/diff/1/?file=1598881#file1598881line109>
> >
> >     formatting

Done.


- Aravindan


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


On Jan. 6, 2017, 10:44 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2017, 10:44 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

Posted by Aravindan Vijayan <av...@hortonworks.com>.

> On Jan. 6, 2017, 9:23 p.m., Sid Wagle wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java, line 124
> > <https://reviews.apache.org/r/55278/diff/1/?file=1598881#file1598881line124>
> >
> >     1. Does this need to be static?
> >     2. Why doesn't it publish an interface instead of abstract impl?

1. Does this need to be static?
The AmbariPerformanceMonitor is instantiated and managed through EclipseLink. For it to reach the DatabaseMetricsSource, I thought that this would be a reasonable method to do that.

2. Why doesn't it publish an interface instead of abstract impl?
Agreed. Will make the change.


- Aravindan


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


On Jan. 6, 2017, 9:01 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2017, 9:01 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>


Re: Review Request 55278: AMBARI-17596 : Collect & Publish AmbariServer database metrics

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




ambari-server/conf/unix/metrics.properties (line 40)
<https://reviews.apache.org/r/55278/#comment231958>

    millis?



ambari-server/conf/unix/metrics.properties (line 46)
<https://reviews.apache.org/r/55278/#comment231960>

    Missing ServiceComponentHost.



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java (line 75)
<https://reviews.apache.org/r/55278/#comment231962>

    Why not drop this, the vars are unambiguous anyways.



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java (line 92)
<https://reviews.apache.org/r/55278/#comment231963>

    This should be a filter method looks like a candidate for extensibility later.



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java (line 112)
<https://reviews.apache.org/r/55278/#comment231965>

    Please add some more doc here preferrably an example.



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java (line 102)
<https://reviews.apache.org/r/55278/#comment231966>

    formatting



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java (line 107)
<https://reviews.apache.org/r/55278/#comment231967>

    formatting



ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java (line 118)
<https://reviews.apache.org/r/55278/#comment231968>

    1. Does this need to be static?
    2. Why doesn't it publish an interface instead of abstract impl?


- Sid Wagle


On Jan. 6, 2017, 9:01 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55278/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2017, 9:01 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-17596
>     https://issues.apache.org/jira/browse/AMBARI-17596
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Extend the eclipse link PeformanceMonitor utility to collect and send metrics to AMS. 
> Counter, Timer and Avg time (Timer/Counter) metrics are tracked.
> 
> The metrics.properties file has enough information to know how to configure the Database metrics source to collect metrics of interest.
> 
> 
> Diffs
> -----
> 
>   ambari-server/conf/unix/metrics.properties 3ee22d6 
>   ambari-server/conf/windows/metrics.properties 3ee22d6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java 4a613f0 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java 400dcb6 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java 6bdd0ba 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java be24988 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java cb9f275 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java ca83a53 
>   ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java d0d2e69 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java 9f649b4 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java 4029f25 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java 3565504 
>   ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java acf1586 
>   ambari-server/src/test/resources/metrics.properties 5eee064 
> 
> Diff: https://reviews.apache.org/r/55278/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests added.
> 
> 
> File Attachments
> ----------------
> 
> Sample Grafana dashboard for AmbariServer DB metrics
>   https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>