Introducing Tracer - Enable logging quickly and precisely !

[Chetan Mehrotra <ch...@gmail.com>]

Hi Team,

Some time back I implemented Tracer [1] which simplifies enabling the
logs for specific category at specific level and only for specific
request. It provides a very fine level of control via config provided
as part of HTTP request around how the logging should be performed for
given category.

For e.g. determining what nodes are written for a given POST request
can be simply done by including an extra request parameter "tracer"

curl -D - -u admin:admin \
 -d "./jcr:content/jcr:title=Summer Collection" \
 -d ":name=summer-collection" \
 -d "./jcr:primaryType=sling:Folder" \
 -d "./jcr:content/jcr:primaryType=nt:unstructured" \
 -d "tracers=oak-writes" \
 http://localhost:8080/content/dam/

One can also specify actual categories as part of request param
itself. For complete details refer to [1]

Initially I submitted this as a pull request to Adobe ACS Tools. After
using it for a while on few customer setups I realize that having it
present out of the box in the product itself would be useful for
developers in there day to day development also. Given that its a
generic component it can be made part of some Sling bundle

Not sure though on whats the best place to host this component. Should
it be part of some new bundle or we add it to [2].

Thoughts?

regards
Chetan
[1] https://github.com/Adobe-Consulting-Services/acs-aem-tools/pull/100
[2] https://github.com/apache/sling/tree/trunk/contrib/extensions/slf4j-mdc

Chetan Mehrotra

Re: Introducing Tracer - Enable logging quickly and precisely !

[Ian Boston <ie...@tfd.co.uk>]

Hi,
Thanks for extracting some of the classes.
 I think you could have gone further, with the Filters as they are service
implementation and being registered inside the activate method does a good
job of hiding them from someone scanning the code base. I don't think it's
important enough to do anything about unless you think it matters.
Best Regards
Ian



On 22 May 2015 at 11:18, Chetan Mehrotra <ch...@gmail.com> wrote:

> Thanks for all the feedback. I have opened SLING-4739 and committed
> the code to svn. The documentation is also moved to Sling site [2]
>
> > For me a Sling- prefix
> > is enough to make it clear that it's a private, Sling-specific,
> > header, but I don't have a strong opinion about it.
>
> @Robert - Code now uses Sling-Tracers and Sling-Tracer-Config has header
> name
>
> > Is there a fundamental reason the LogTracer class must put ServletFilters
> > as inner classes  ? It has 8 inner classes which generally makes
> > understanding the code harder and leads to the temptation to reference
> > internal implementation details across class boundaries. I understand
> other
> > languages have that flexibility by mistake or intention, but the pattern
> > when used in Java tends to lead to problems downstream.
>
> @Ian - I prefer use of inner class if the class is not to be exposed
> or not to be known to outer world. However I agree in this case it
> went bit overboard, so committed code is refactored to simplify the
> logic. Hopefully this is simpler!
>
> Chetan Mehrotra
> [1] https://issues.apache.org/jira/browse/SLING-4739
> [2] http://sling.apache.org/documentation/bundles/log-tracers.html
>
>
> On Thu, May 21, 2015 at 2:11 PM, Robert Munteanu <ro...@apache.org>
> wrote:
> > On Thu, May 21, 2015 at 11:38 AM, Chetan Mehrotra
> > <ch...@gmail.com> wrote:
> >> On Thu, May 21, 2015 at 2:00 PM, Robert Munteanu <ro...@apache.org>
> wrote:
> >>> AEM -> Sling is self-explanatory,  and the X- prefix is deprecated with
> >>> RFC6648 [1]
> >>
> >> Well not sure. That recommendation is meant for headers which are
> >> meant to be used in a wider context. Header which are very much
> >> specific to application are better prefixed to indicate that they are
> >> custom ones IMHO [2]. So I would prefer naming them X-Sling-Tracers.
> >
> > Right, it doesn't apply to 'private' headers. For me a Sling- prefix
> > is enough to make it clear that it's a private, Sling-specific,
> > header, but I don't have a strong opinion about it.
> >
> > Robert
>

Re: Introducing Tracer - Enable logging quickly and precisely !

[Chetan Mehrotra <ch...@gmail.com>]

Thanks for all the feedback. I have opened SLING-4739 and committed
the code to svn. The documentation is also moved to Sling site [2]

> For me a Sling- prefix
> is enough to make it clear that it's a private, Sling-specific,
> header, but I don't have a strong opinion about it.

@Robert - Code now uses Sling-Tracers and Sling-Tracer-Config has header name

> Is there a fundamental reason the LogTracer class must put ServletFilters
> as inner classes  ? It has 8 inner classes which generally makes
> understanding the code harder and leads to the temptation to reference
> internal implementation details across class boundaries. I understand other
> languages have that flexibility by mistake or intention, but the pattern
> when used in Java tends to lead to problems downstream.

@Ian - I prefer use of inner class if the class is not to be exposed
or not to be known to outer world. However I agree in this case it
went bit overboard, so committed code is refactored to simplify the
logic. Hopefully this is simpler!

Chetan Mehrotra
[1] https://issues.apache.org/jira/browse/SLING-4739
[2] http://sling.apache.org/documentation/bundles/log-tracers.html


On Thu, May 21, 2015 at 2:11 PM, Robert Munteanu <ro...@apache.org> wrote:
> On Thu, May 21, 2015 at 11:38 AM, Chetan Mehrotra
> <ch...@gmail.com> wrote:
>> On Thu, May 21, 2015 at 2:00 PM, Robert Munteanu <ro...@apache.org> wrote:
>>> AEM -> Sling is self-explanatory,  and the X- prefix is deprecated with
>>> RFC6648 [1]
>>
>> Well not sure. That recommendation is meant for headers which are
>> meant to be used in a wider context. Header which are very much
>> specific to application are better prefixed to indicate that they are
>> custom ones IMHO [2]. So I would prefer naming them X-Sling-Tracers.
>
> Right, it doesn't apply to 'private' headers. For me a Sling- prefix
> is enough to make it clear that it's a private, Sling-specific,
> header, but I don't have a strong opinion about it.
>
> Robert

Re: Introducing Tracer - Enable logging quickly and precisely !

[Robert Munteanu <ro...@apache.org>]

On Thu, May 21, 2015 at 11:38 AM, Chetan Mehrotra
<ch...@gmail.com> wrote:
> On Thu, May 21, 2015 at 2:00 PM, Robert Munteanu <ro...@apache.org> wrote:
>> AEM -> Sling is self-explanatory,  and the X- prefix is deprecated with
>> RFC6648 [1]
>
> Well not sure. That recommendation is meant for headers which are
> meant to be used in a wider context. Header which are very much
> specific to application are better prefixed to indicate that they are
> custom ones IMHO [2]. So I would prefer naming them X-Sling-Tracers.

Right, it doesn't apply to 'private' headers. For me a Sling- prefix
is enough to make it clear that it's a private, Sling-specific,
header, but I don't have a strong opinion about it.

Robert

Re: Introducing Tracer - Enable logging quickly and precisely !

[Chetan Mehrotra <ch...@gmail.com>]

On Thu, May 21, 2015 at 2:00 PM, Robert Munteanu <ro...@apache.org> wrote:
> AEM -> Sling is self-explanatory,  and the X- prefix is deprecated with
> RFC6648 [1]

Well not sure. That recommendation is meant for headers which are
meant to be used in a wider context. Header which are very much
specific to application are better prefixed to indicate that they are
custom ones IMHO [2]. So I would prefer naming them X-Sling-Tracers.

Chetan Mehrotra
[2] http://stackoverflow.com/a/19640336/1035417

Re: Introducing Tracer - Enable logging quickly and precisely !

[Robert Munteanu <ro...@apache.org>]

On Thu, 2015-05-21 at 10:04 +0200, Bertrand Delacretaz wrote:
> Hi,
> 
> On Thu, May 21, 2015 at 8:03 AM, Ian Boston <ie...@tfd.co.uk> wrote:
> > ...Putting this in Sling would benefit Sling and a wider community, 
> > +1 on
> > inclusion...
> 
> +1, under contrib/extensions for now I suppose.

+1

Only only took a brief look at the documentation in the pull request,
but I noticed the request headers docs, which mention the custom 'X-AEM
-Tracers' and 'X-AEM-Tracer-Config' headers. This should be chagend to
Sling-Tracers and Sling-Tracer-Config .

AEM -> Sling is self-explanatory,  and the X- prefix is deprecated with
RFC6648 [1]

Cheers,

Robert


[1]: http://tools.ietf.org/html/rfc6648

Re: Introducing Tracer - Enable logging quickly and precisely !

[Bertrand Delacretaz <bd...@apache.org>]

Hi,

On Thu, May 21, 2015 at 8:03 AM, Ian Boston <ie...@tfd.co.uk> wrote:
> ...Putting this in Sling would benefit Sling and a wider community, +1 on
> inclusion...

+1, under contrib/extensions for now I suppose.

-Bertrand

Re: Introducing Tracer - Enable logging quickly and precisely !

[Ian Boston <ie...@tfd.co.uk>]

Hi,
Putting this in Sling would benefit Sling and a wider community, +1 on
inclusion.

 Is there a fundamental reason the LogTracer class must put ServletFilters
as inner classes  ? It has 8 inner classes which generally makes
understanding the code harder and leads to the temptation to reference
internal implementation details across class boundaries. I understand other
languages have that flexibility by mistake or intention, but the pattern
when used in Java tends to lead to problems downstream.

Best Regards
Ian

On 21 May 2015 at 04:47, Chetan Mehrotra <ch...@gmail.com> wrote:

> Hi Team,
>
> Some time back I implemented Tracer [1] which simplifies enabling the
> logs for specific category at specific level and only for specific
> request. It provides a very fine level of control via config provided
> as part of HTTP request around how the logging should be performed for
> given category.
>
> For e.g. determining what nodes are written for a given POST request
> can be simply done by including an extra request parameter "tracer"
>
> curl -D - -u admin:admin \
>  -d "./jcr:content/jcr:title=Summer Collection" \
>  -d ":name=summer-collection" \
>  -d "./jcr:primaryType=sling:Folder" \
>  -d "./jcr:content/jcr:primaryType=nt:unstructured" \
>  -d "tracers=oak-writes" \
>  http://localhost:8080/content/dam/
>
> One can also specify actual categories as part of request param
> itself. For complete details refer to [1]
>
> Initially I submitted this as a pull request to Adobe ACS Tools. After
> using it for a while on few customer setups I realize that having it
> present out of the box in the product itself would be useful for
> developers in there day to day development also. Given that its a
> generic component it can be made part of some Sling bundle
>
> Not sure though on whats the best place to host this component. Should
> it be part of some new bundle or we add it to [2].
>
> Thoughts?
>
> regards
> Chetan
> [1] https://github.com/Adobe-Consulting-Services/acs-aem-tools/pull/100
> [2]
> https://github.com/apache/sling/tree/trunk/contrib/extensions/slf4j-mdc
>
> Chetan Mehrotra
>