You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Chetan Mehrotra <ch...@gmail.com> on 2016/01/05 16:54:55 UTC

[RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Hi Team,

Reviving the old thread [1] around introducing an API to collect
Metric data using Metric library [2].

Recently as part of OAK-3814 in Jackrabbit Oak we added support for
collecting important runtime performance data using the Metric library
and it looks useful. Based on that experience I think it would be good
to have a similar API in Sling also. I have done initial
implementation at [3] which provides a simple wrapper API over Metrics
library

Design Points
==========

1. Expose the sub set of Metric API which is useful for majority code
which just need to collect data - Metric API has both data collection
and data reporting api. Former is the one which would be used by
majority of code while later is used only for tooling. So the api
proposed in [3] is subset of Metric API

2. Provide a NOOP implementation so as to make Metric integration
optional - As the new Metrics support would be getting used in core
areas of Sling which so far do not have any dependency. It should be
possible to have a simple NOOP implementation which can be inlined
into using bundle (if required). It would also allow us to turn of
Metric integration easily so as to access impact of monitoring (if
any/or if it gives problem)

3. WebConsole Integration - Metric bundle exposes a WebConsole plugin
[4] and printer to dump all metric data in a single place

4. Support for Adapting to underlying Metric instance - One can access
the Metric instance if required

TODO
======

1. Have TimeSeries support - Would be good to have something similar
to Jackrabbit TimeSeries [6] in Sling. It would allow us to view the
data as it updated in time. This can be easily plugged in later with
current design

Usage
=====

API is very much similar to [5]

---------------------------------------------------------------------------

import org.apache.sling.metrics.Counter;
import org.apache.sling.metrics.MetricsService;

@Reference
private MetricsService metricsService;

private Counter counter;

@Activate
private void activate(){
    counter = metricsService.counter("sessionCounter");
}

public void onSessionCreation(){
    counter.inc();
}
---------------------------------------------------------------------------

1. Get a reference to MetricsService
2. Initialize the require Meter e.g. Counter in above case
3. Make use of Meter to capture require stats

Kindly provide your feedback here or in SLING-4080!

Also should we move the code now to commons from whiteboard?

Chetan Mehrotra
[1] http://markmail.org/thread/cyjc2v34l3lxrtxb
[2] http://metrics.dropwizard.io/
[3] https://svn.apache.org/repos/asf/sling/whiteboard/chetanm/metrics/src/main/java/org/apache/sling/metrics/
[4] https://issues.apache.org/jira/secure/attachment/12780565/metric-web-console.png
[5] http://metrics.dropwizard.io/3.1.0/getting-started/#counters
[6] https://github.com/apache/jackrabbit/blob/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/stats/TimeSeries.java

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Ian Boston <ie...@tfd.co.uk>.
Hi Chetan,
Thank you for picking up SLING-4080 and the thread which stalled some time
ago. I am +1 on getting it into commons so it Sling can be instrumented in
a way that is safe to use in production 24*7. Very good to hear the same
has been done in Oak. Apologies for having been unable to help more with
this effort, being a bit snowed under at present.
Best Regards
Ian

On 5 January 2016 at 15:54, Chetan Mehrotra <ch...@gmail.com>
wrote:

> Hi Team,
>
> Reviving the old thread [1] around introducing an API to collect
> Metric data using Metric library [2].
>
> Recently as part of OAK-3814 in Jackrabbit Oak we added support for
> collecting important runtime performance data using the Metric library
> and it looks useful. Based on that experience I think it would be good
> to have a similar API in Sling also. I have done initial
> implementation at [3] which provides a simple wrapper API over Metrics
> library
>
> Design Points
> ==========
>
> 1. Expose the sub set of Metric API which is useful for majority code
> which just need to collect data - Metric API has both data collection
> and data reporting api. Former is the one which would be used by
> majority of code while later is used only for tooling. So the api
> proposed in [3] is subset of Metric API
>
> 2. Provide a NOOP implementation so as to make Metric integration
> optional - As the new Metrics support would be getting used in core
> areas of Sling which so far do not have any dependency. It should be
> possible to have a simple NOOP implementation which can be inlined
> into using bundle (if required). It would also allow us to turn of
> Metric integration easily so as to access impact of monitoring (if
> any/or if it gives problem)
>
> 3. WebConsole Integration - Metric bundle exposes a WebConsole plugin
> [4] and printer to dump all metric data in a single place
>
> 4. Support for Adapting to underlying Metric instance - One can access
> the Metric instance if required
>
> TODO
> ======
>
> 1. Have TimeSeries support - Would be good to have something similar
> to Jackrabbit TimeSeries [6] in Sling. It would allow us to view the
> data as it updated in time. This can be easily plugged in later with
> current design
>
> Usage
> =====
>
> API is very much similar to [5]
>
> ---------------------------------------------------------------------------
>
> import org.apache.sling.metrics.Counter;
> import org.apache.sling.metrics.MetricsService;
>
> @Reference
> private MetricsService metricsService;
>
> private Counter counter;
>
> @Activate
> private void activate(){
>     counter = metricsService.counter("sessionCounter");
> }
>
> public void onSessionCreation(){
>     counter.inc();
> }
> ---------------------------------------------------------------------------
>
> 1. Get a reference to MetricsService
> 2. Initialize the require Meter e.g. Counter in above case
> 3. Make use of Meter to capture require stats
>
> Kindly provide your feedback here or in SLING-4080!
>
> Also should we move the code now to commons from whiteboard?
>
> Chetan Mehrotra
> [1] http://markmail.org/thread/cyjc2v34l3lxrtxb
> [2] http://metrics.dropwizard.io/
> [3]
> https://svn.apache.org/repos/asf/sling/whiteboard/chetanm/metrics/src/main/java/org/apache/sling/metrics/
> [4]
> https://issues.apache.org/jira/secure/attachment/12780565/metric-web-console.png
> [5] http://metrics.dropwizard.io/3.1.0/getting-started/#counters
> [6]
> https://github.com/apache/jackrabbit/blob/trunk/jackrabbit-api/src/main/java/org/apache/jackrabbit/api/stats/TimeSeries.java
>

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Julian Sedding <js...@gmail.com>.
Agreed ;) As always, good judgement is required. Hiding the NOOP
implementation (in an OSGi world) sounds sensible.

Regards
Julian

On Wed, Jan 6, 2016 at 5:46 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> Julian Sedding wrote
>>> Testing should not drive api design :)
>>
>> Really?! I thought that was what test driven design was all about...
>>
> Oh well...sure. Point is, adding stuff to an api just to make someone
> else's tests easier doesn't sound like a good idea.
>
> Carsten
> --
> Carsten Ziegeler
> Adobe Research Switzerland
> cziegeler@apache.org

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Carsten Ziegeler <cz...@apache.org>.
Julian Sedding wrote
>> Testing should not drive api design :)
> 
> Really?! I thought that was what test driven design was all about...
> 
Oh well...sure. Point is, adding stuff to an api just to make someone
else's tests easier doesn't sound like a good idea.

Carsten
-- 
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Julian Sedding <js...@gmail.com>.
> Testing should not drive api design :)

Really?! I thought that was what test driven design was all about...

Julian

On Wed, Jan 6, 2016 at 5:23 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> Chetan Mehrotra wrote
>>> - I don't see a reason to have NoopMetric and MetricsService.NOOP in the API.
>>
>> In Oak I have found it useful to have NOOP in API for code to evolve.
>> Some examples
>>
>> 1. Allow use of Metrics in code Sling bundle - If we add a direct
>> dependency on Metrics then any bundle if to be deployed on older setup
>> would also pull in metrics bundle and Metrics library. Having NOOP
>> would allow such bundles to have a optional dependency on
>> MetricsService and default to NOOP (which can be easily inlined and
>> reimported) and let them be usable in older setups
>
> Hmm...ok, that allows for nicer code than doing a null check all over
> the place.
>
>>
>> 2. It also enables unit testing - One can stub out the MetricsService
>> using NOOP implementation to test the logic without pulling in
>> complete implementation
>
> Testing should not drive api design :)
>
>> Also note that NoopMetric is package protected. So not exposed
>
> But it appears in the api as a main class. Can we maybe hide it and
> inline it in MetricsService?
>
>>
>>> what happens if a creation method on the MetricsService for the same name is called twice?
>>
>> The service is thread safe and same object would be reused. So yes it
>> can hide duplicates. One idea I had was to use a ServiceFactory and
>> detect such duplicates there based on calling bundle
>
> Ok, so the javadocs need to be adjusted that you either get a new or an
> existing one :)
>
>>
>>> what about an api to get all metrics or a specific one?
>>
>> The focus for wrapper API was collection side as that would be used by
>> majority of classes. For getting all Metrics one can directly access
>> the underlying MetricRegistry by using dropwizard Metrics library API
>
> I see...ok, this api is purely for generating metrics - which makes
> totally sense.
>
> Regards
> Carsten
>
>
>
>
> --
> Carsten Ziegeler
> Adobe Research Switzerland
> cziegeler@apache.org

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Chetan Mehrotra <ch...@gmail.com>.
Thank to all for the feedback! I have moved the code to commons. Would
now get it to finish state and cut the initial release soon

Next Step - What types of Metrics should we collect in Sling? Would
start a new thread for that
Chetan Mehrotra


On Thu, Jan 7, 2016 at 10:18 AM, Chetan Mehrotra
<ch...@gmail.com> wrote:
> On Wed, Jan 6, 2016 at 9:53 PM, Carsten Ziegeler <cz...@apache.org> wrote:
>> Hmm...ok, that allows for nicer code than doing a null check all over the place.
>
> Yes thats the purpose.
>
>> But it appears in the api as a main class. Can we maybe hide it and
>> inline it in MetricsService?
>
> Can be done but it would add unnecessary noise to MetricService
> interface source file. Given its package protected it should be ok.
>
>> Ok, so the javadocs need to be adjusted that you either get a new or an existing one :)
>
> Ack. Would do that
>
> Chetan Mehrotra

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Chetan Mehrotra <ch...@gmail.com>.
On Wed, Jan 6, 2016 at 9:53 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> Hmm...ok, that allows for nicer code than doing a null check all over the place.

Yes thats the purpose.

> But it appears in the api as a main class. Can we maybe hide it and
> inline it in MetricsService?

Can be done but it would add unnecessary noise to MetricService
interface source file. Given its package protected it should be ok.

> Ok, so the javadocs need to be adjusted that you either get a new or an existing one :)

Ack. Would do that

Chetan Mehrotra

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Carsten Ziegeler <cz...@apache.org>.
Chetan Mehrotra wrote
>> - I don't see a reason to have NoopMetric and MetricsService.NOOP in the API.
> 
> In Oak I have found it useful to have NOOP in API for code to evolve.
> Some examples
> 
> 1. Allow use of Metrics in code Sling bundle - If we add a direct
> dependency on Metrics then any bundle if to be deployed on older setup
> would also pull in metrics bundle and Metrics library. Having NOOP
> would allow such bundles to have a optional dependency on
> MetricsService and default to NOOP (which can be easily inlined and
> reimported) and let them be usable in older setups

Hmm...ok, that allows for nicer code than doing a null check all over
the place.

> 
> 2. It also enables unit testing - One can stub out the MetricsService
> using NOOP implementation to test the logic without pulling in
> complete implementation

Testing should not drive api design :)

> Also note that NoopMetric is package protected. So not exposed

But it appears in the api as a main class. Can we maybe hide it and
inline it in MetricsService?

> 
>> what happens if a creation method on the MetricsService for the same name is called twice?
> 
> The service is thread safe and same object would be reused. So yes it
> can hide duplicates. One idea I had was to use a ServiceFactory and
> detect such duplicates there based on calling bundle

Ok, so the javadocs need to be adjusted that you either get a new or an
existing one :)

> 
>> what about an api to get all metrics or a specific one?
> 
> The focus for wrapper API was collection side as that would be used by
> majority of classes. For getting all Metrics one can directly access
> the underlying MetricRegistry by using dropwizard Metrics library API

I see...ok, this api is purely for generating metrics - which makes
totally sense.

Regards
Carsten



 
-- 
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Chetan Mehrotra <ch...@gmail.com>.
> - I don't see a reason to have NoopMetric and MetricsService.NOOP in the API.

In Oak I have found it useful to have NOOP in API for code to evolve.
Some examples

1. Allow use of Metrics in code Sling bundle - If we add a direct
dependency on Metrics then any bundle if to be deployed on older setup
would also pull in metrics bundle and Metrics library. Having NOOP
would allow such bundles to have a optional dependency on
MetricsService and default to NOOP (which can be easily inlined and
reimported) and let them be usable in older setups

2. It also enables unit testing - One can stub out the MetricsService
using NOOP implementation to test the logic without pulling in
complete implementation

> what happens if a creation method on the MetricsService for the same name is called twice?

The service is thread safe and same object would be reused. So yes it
can hide duplicates. One idea I had was to use a ServiceFactory and
detect such duplicates there based on calling bundle

> what about an api to get all metrics or a specific one?

The focus for wrapper API was collection side as that would be used by
majority of classes. For getting all Metrics one can directly access
the underlying MetricRegistry by using dropwizard Metrics library API
Chetan Mehrotra


On Wed, Jan 6, 2016 at 9:10 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> Lgtm, thanks Chetan, so +1 :)
>
> Some comments:
>
> - I don't see a reason to have NoopMetric and MetricsService.NOOP in the
> API.
> - what happens if a creation method on the MetricsService for the same
> name is called twice?
> - what about an api to get all metrics or a specific one?
>
> Regards
> Carsten
> --
> Carsten Ziegeler
> Adobe Research Switzerland
> cziegeler@apache.org

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Carsten Ziegeler <cz...@apache.org>.
Yes, let's include commons in both names

Carsten

Chetan Mehrotra wrote
> Confirmation for API name
> 
> Current state
> 1. Bundle name - org.apache.sling.metrics
> 2. Package name - org.apache.sling.metrics
> 
> So questions
> 
> Q1. Should this bundle go to bundle/commons
> Q2. Should 'commons' be added to bundle name and package name. Other
> API in commons have commons? So they change to
> org.apache.sling.commons.metrics
> Chetan Mehrotra
> 
> 
> On Wed, Jan 6, 2016 at 9:36 PM, Chetan Mehrotra
> <ch...@gmail.com> wrote:
>> On Wed, Jan 6, 2016 at 9:10 PM, Carsten Ziegeler <cz...@apache.org> wrote:
>>> I don't see a reason to have NoopMetric and MetricsService.NOOP in the
>>> API.
>>
>> Also note that NoopMetric is package protected. So not exposed
>>
>> Chetan Mehrotra
> 


 
-- 
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Chetan Mehrotra <ch...@gmail.com>.
Confirmation for API name

Current state
1. Bundle name - org.apache.sling.metrics
2. Package name - org.apache.sling.metrics

So questions

Q1. Should this bundle go to bundle/commons
Q2. Should 'commons' be added to bundle name and package name. Other
API in commons have commons? So they change to
org.apache.sling.commons.metrics
Chetan Mehrotra


On Wed, Jan 6, 2016 at 9:36 PM, Chetan Mehrotra
<ch...@gmail.com> wrote:
> On Wed, Jan 6, 2016 at 9:10 PM, Carsten Ziegeler <cz...@apache.org> wrote:
>> I don't see a reason to have NoopMetric and MetricsService.NOOP in the
>> API.
>
> Also note that NoopMetric is package protected. So not exposed
>
> Chetan Mehrotra

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Chetan Mehrotra <ch...@gmail.com>.
On Wed, Jan 6, 2016 at 9:10 PM, Carsten Ziegeler <cz...@apache.org> wrote:
> I don't see a reason to have NoopMetric and MetricsService.NOOP in the
> API.

Also note that NoopMetric is package protected. So not exposed

Chetan Mehrotra

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Carsten Ziegeler <cz...@apache.org>.
Lgtm, thanks Chetan, so +1 :)

Some comments:

- I don't see a reason to have NoopMetric and MetricsService.NOOP in the
API.
- what happens if a creation method on the MetricsService for the same
name is called twice?
- what about an api to get all metrics or a specific one?

Regards
Carsten
-- 
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi,

On Wed, Jan 6, 2016 at 12:03 PM, Chetan Mehrotra
<ch...@gmail.com> wrote:
> On Wed, Jan 6, 2016 at 3:45 PM, Bertrand Delacretaz
> <bd...@apache.org> wrote:
>> Would that allow one to access the Oak and Sling metrics in the same
>> way, in the webconsole and/or JMX? Or are those disconnected sets of
>> metrics?
>
> On reporting side i.e. WebConsole plugin and Config printer it would
> list metric data from all MetricRegistry present in ServiceRegistry.
> Oak publishes its MetricRegistry instance so that would be picked up
> also. JMX is anyway common to both....

Great, so definite +1 for moving that under commons.

-Bertrand

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Chetan Mehrotra <ch...@gmail.com>.
On Wed, Jan 6, 2016 at 3:45 PM, Bertrand Delacretaz
<bd...@apache.org> wrote:
> Would that allow one to access the Oak and Sling metrics in the same
> way, in the webconsole and/or JMX? Or are those disconnected sets of
> metrics?

On reporting side i.e. WebConsole plugin and Config printer it would
list metric data from all MetricRegistry present in ServiceRegistry.
Oak publishes its MetricRegistry instance so that would be picked up
also. JMX is anyway common to both.

> MetricsService mentions types like com.codahale.metrics.Timer but IIUC it returns Sling types, not those.

Ah. Yup thats a mistake. Fixed that now

Chetan Mehrotra

Re: [RTC] SLING-4080 - API to capture/measure application-level metrics (update)

Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi Chetan,

On Tue, Jan 5, 2016 at 4:54 PM, Chetan Mehrotra
<ch...@gmail.com> wrote:
> ...Recently as part of OAK-3814 in Jackrabbit Oak we added support for
> collecting important runtime performance data using the Metric library...
> ...I have done initial implementation at [3] which provides a simple wrapper API
> over Metrics library...

I had a quick look, looks good to me!

Would that allow one to access the Oak and Sling metrics in the same
way, in the webconsole and/or JMX? Or are those disconnected sets of
metrics?

-Bertrand

P.S. nitpick: MetricsService mentions types like
com.codahale.metrics.Timer but IIUC it returns Sling types, not those.