Add metrics for JDBC connection pool: Apache Commons DBCP2 #144
Conversation
| () -> | ||
| assertThat(TestMetricsAccess.getMeterNames()) | ||
| .containsExactlyInAnyOrder( | ||
| "db.pool.connections.active", |
There was a problem hiding this comment.
Meter tags/units are not being verified yet, I'm planning to implement that in the next PR
...ion/commons-dbcp2/src/main/java/com/splunk/opentelemetry/commonsdbcp2/DataSourceMetrics.java
Outdated
Show resolved
Hide resolved
...ion/commons-dbcp2/src/main/java/com/splunk/opentelemetry/commonsdbcp2/DataSourceMetrics.java
Outdated
Show resolved
Hide resolved
|
have we verified that these metrics make it to the splunk backends and are properly chartable, etc? |
...2/src/main/java/com/splunk/opentelemetry/commonsdbcp2/CommonsDbcp2InstrumentationModule.java
Outdated
Show resolved
Hide resolved
|
| public static void registerMetrics(BasicDataSourceMXBean dataSource, ObjectName objectName) { | ||
| List<Tag> tags = getTags(objectName); | ||
|
|
||
| gauge(CONNECTIONS_TOTAL, tags, new TotalConnectionsUsed(dataSource)); |
There was a problem hiding this comment.
Are these unregistered somewhere? If not does it mean that when the application that added these metrics is undeployed we'll still hold a reference to them?
There was a problem hiding this comment.
Oh, that is a very good point -- I'll fix that in another PR. Thanks!
| @Override | ||
| public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() { | ||
| return singletonMap( | ||
| isPublic().and(named("preRegister")).and(takesArguments(2)), |
There was a problem hiding this comment.
I assume this means that metrics are only collected when jmx bean is registered. I was just curious is registering the jmx bean a default behaviour? Any idea whether users actually register it?
There was a problem hiding this comment.
In a Spring application Spring does it automatically -- for manually created and managed connection pools you have to set the pool's JMX name for it to be registered (which is not set by default).
I think it was reasonable to hook on the preRegister method, since it provides us a unique object name - even if for some users (which I suspect won't be many) it won't ever execute.
BasicDataSource#jmxNamebeing set (BasicDataSourceadds itself to the mbean server when you set the JMX name), unfortunately Spring completely ignores that property and registers the data source MXBean by itself. This results in the underlying object pool (andPooledConnection) MXBeans not being registered, because they're not spring beans - just objects created insideBasicDataSource. Because of that I had to ignore the commons-pool2 instrumentation and implement metrics just for dbcp2.BasicDataSourceMXBean) in this instrumentation - using plain JMX without compileOnly dependency is possible, but it's considerably less readable - you can see the example in the micrometerCommonsObjectPool2Metricsclass.