Timer-per-query

[mwanji]

Could we have separate metrics per query?

For example, SELECT * FROM users and SELECT * FROM accounts would be measured by different timers (along with being aggregated into more general timers).

This seems feasible, as the InvocationHandlers know the SQL being executed, but is it desirable (maybe as an optional setting)?

[soulgalore]

Well, I would love to have it but think it is hard to implement in a good way? When @mlundberg & me discussed it before, we thought we maybe would add a log for each query, as a way to at least get hold of the info?

I mean, the problem as I see it is to group "the same" question: Prepared statement with an ? for the value is in one way easy, but what is the best way to handle queries with SELECT .. IN and the like? will each be a unique query? And queries with the actual value in them, how do we group them? Thinking that for an application, many queries are unique by the value but somewhat the same. That logic is the hard part, how to group them.

How do you see it @mwanji ? Maybe I just overcomplicate things?

[mwanji]

Those are good points to think about. A generic solution cannot be perfect, especially as you don't want to add the overhead of parsing SQL to be able to better group it. However, I think the usecase of wanting query-level stats is strong enough to warrant at least an attempt. Otherwise, it is up to the user to wrap every SQL call in a Timer, which is annoying and verbose (even in Java 7's try-with-resources), especially if you want to isolate the SQL part from the in-memory processing.

I would suggest having this off by default, and perhaps marking it experimental, so that APIs may change if necessary. When enabled, by default the grouping is simply by SQL string (no handling of IN or other forms of dynamic SQL, such as WHERE clauses based on user input).

Perhaps there could be an interface users could implement to provide such grouping themselves? For example, a method would receive the SQL and return the name of the timer to use. That could solve the problem of naming the timers at the same time, as potentially very long SQL statements don't make for great names.

[soulgalore]

Yes I see your point, as long as it is turned off by default it sounds ok. The thing is that @mlundberg just got his second kid and I'm home on parental leave so we don't have so much extra time. If you have the time, I would gladly make you a collaborator and you can implement it?

[mlundberg]

I think the interface/grouping sounds like an exciting idea, and I also think the read/write metrics should be handle in the same way, though some major refactoring is needed. Also a future query log could maybe be handled in the same way.

A thought: StatementInvocationHandler could call configured "MetricsDeterminer" classes with needed params (as Method (execute, executeQuery etc), sql string, params, anything else?). In return we get the metrics counter to use (or just the name).

Is there a need to exclude a query from another counter? In that case we need this as some kind of chain perhaps, but I think there is no need. So the "determiners" could be independent from each other.

[mwanji]

I totally understand about the kids, I have two young ones of my own.

I suggest releasing the current code as 2.0, if you're happy with it, and then the timer-per-query and interface stuff can come in 2.1, with no time pressure.

When I get the time, I'll start a feature branch for this and you guys can comment, contribute, whatever.

[soulgalore]

guess all three of us have two kids then :)

yep sounds good, I'll will do the 2.0 release in a couple of days then.

[mwanji]

Any time frame for a 2.0 release? It's starting to become a serious issue for me.

[soulgalore]

@mwanji ah sorry yes, can try to do it today, sorry

[soulgalore]

so now is it at Sonatype, sorry for the delay.

[mwanji]

Cool, thanks.

[mwanji]

I've created a branch for this on my fork: https://github.com/mwanji/jdbcmetrics/tree/timer_per_query and a repo for tests: https://github.com/mwanji/jdbcmetrics-test

It's a very rudimentary change so far, a naïve implementation of measuring read queries, just to get some feedback on API changes and approach.

[soulgalore]

Ah cool, I will have a look in a couple of days.