Metrics follow-up thoughts

[Brent Putman]

On 9/16/16 2:52 PM, Cantor, Scott wrote:
> A few ruminations after the dev call.

I was just reviewing the Metrics "Getting Started" stuff.

Metrics itself seems pretty good (not that I'm terribly familiar with
this space).  It's pretty simple and the docs are pretty clear.


> It's worth noting that supporting Metrics itself doesn't preclude people doing outside-in measuring with other tools, particularly if it's easy to turn off the Metrics piece. I also think that's a much better way of addressing *performance monitoring* as distinct from behavioral/statistical analysis or reporting on internal configuration state.

I was actually next just looking into that very topic.

New Relic and I imagine many similar APM things use something that I
was not really informed much about called (somewhat confusing) "Java
agents", which you add to the JVM with an arg like "-javaagent
agent.jar" (ala New Relic).  It's been in Java for quite a long time. 
It's basically a pre-built way to have Java transform the byte code at
runtime, for example to add instrumentation, although I think it's more
general than that.  Several references:

http://javabeat.net/introduction-to-java-agents/
http://javahowto.blogspot.com/2006/07/javaagent-option.html
http://blog.takipi.com/double-agent-java-vs-native-agents/

I was thinking that it would be theoretically possible to actually do a
hybrid as Scott suggested and use Metrics (or whatever) in an
outside-in type of way via a -javaagent.  Looks like this project
already does it, might be worth a look.  Haven't read in detail, but
looks like they might support their own annotations for configuring things:

https://github.com/willfleury/metrics-agent

And here there's some actual code for a hypothetical agent impl,
although it's a bit hard to read because of all the HTML escaping:

https://chimpler.wordpress.com/2013/11/05/implementing-a-java-agent-to-instrument-code/

I'm definitely not saying we have to do anything like that.  This
approach achieves non-invasiveness at the cost of real technological
sophistication and complexity, which probably requires a great deal
more expert knowledge than merely adding Metrics stuff directly to a class.

Mostly just wanted to point out that we are definitely not the first
people to think about stuff like this.

> On the subject of turning this on/off, I wonder if there's a possibility of leveraging the logging API to tune the behavior of any measuring code, whether it's specifically using the Metrics API or not. Couldn't it be acting based on the level of a matching logging category?

That's an interesting idea, hadn't thought about reusing the logger
hierarchy.

I was thinking about something conceptually similar to Loggers and
LoggerFactory, where instead of interacting with the Metrics classes
directly, you did so through a support class (like LoggerFactory),
which sort of mirrored the important methods of MetricRegistry.  If
metrics was turned off, it would return objects that just did no-ops,
etc.  Or something like that.

At the level of OpenSAML, one way around the need to inject
MetricRegistry everywhere would be so store it in the
ConfigurationService, which by default exposes things essentially in a
singleton-like way.  The IdP could inject the registry there for
example, and instrumented code then pulls from there, probably
indirectly via a support class like XMLObjectProviderRegistry.  Then
treat that if not present, metrics is disabled.   Evaling where enabled
that could be in the support class mentioned above.

>
> This is a bad example because it's too lightweight to bother turning off, but if you had a Counter metric called "authn.ldap.failure" in the "net.shibboleth.idp.metric" metric registry, you could check a logger for "net.shibboleth.idp.metric.authn.ldap.failure" and then:
>
> if (metriclevel.isInfoEnabled()) {
> 	failures.inc();
> }
>
> You could even have discrete log levels assigned to different measuring code based on the cost so you'd get general performance impacts based on the log level for net.shibboleth.idp.metric.
>
> That makes the tuning reloadable, of course, since the log levels are.

That granularity and hierarchical config does seem nice to have, if it
can be made to work.


>
> Separately...
>
> A consideration with some of this is that our actions can't actually be maintaining their own metric objects anyway, since they're prototypes.

Hmm, yeah, good point.  I thought they mentioned stuff about being
thread-safe everywhere, but maybe it doesn't work if you are literally
trying to register an existing name.  At least the ones like
counter(...) though use a getOrAdd approach, so I think those might
actually work within the prototypes.  Maybe for the register(...), e.g.
of a Guage, they have a way to get an existing one rather than create a
new one.  They do have Maps of all these things on the registry.


>  That does complicate a lot of this no matter how we do it, since that means in a lot of cases, there's no way to track the information in a way that's internal to the actions that might be recording it. The authn case is an example of that. We have no choice but to store the data independently of the actions. So either we just build a collector class ourselves for that data, or we inject a Metrics object into the action(s) to maintain it, or they access the global shared object to get the metric by name (in which case we have to tell it the name).

Or perhaps the prototype beans get wrapped in proxies by Spring bean
post processor.  Or there's always proxies via AOP byte code
manipulation type stuff.  I imagine a hugely common case is just adding
something before and/or after a method call.  Just brainstorming.


>
> As I think I aluded to on the call, mostly for Brent, I think it would be useful to consider the Dynamic resolver code as an example where there are a number of metrics we'd want to be able to report, and since that's in OpenSAML, it would be a good case study for how this should be handled. Maybe that would help point the way towards a way of insulating the IdP code more than I had envisioned.

Certainly fine to focus on that as a prime use case.