Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable Metrics prefix #624

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

EdgeCaseBerg
Copy link

@EdgeCaseBerg EdgeCaseBerg commented Apr 30, 2016

Hi! I saw issue 615 and figure'd it would be something pretty easy to do in a morning.
All the tests still pass, and I've written the update so that it wouldn't break anyone's metrics
that are relying on the hardcoded "pool" being added to the pool name as the base of the
metrics name.

Adding `metricsPrefix` field to HikariConfig and a default of "pool"
because that's what it is currently. This is work towards brettwooldridge#615

Setting the metricsPrefix by default to the poolname + "pool" so that it
matches what's going on in the CodahaleHealthChecker and it will be
easier to update that and keep the old behavior for other people who
don't need to set a custom prefix on there metrics.
Updated metrics related classes to use the metricsPrefix from
HikariConfig rather than a combination of the poolname (from config)
plus the hard coded word "pool". This gives more control to users who
need to set the prefixes of their metrics to something specific such as
the person who raised brettwooldridge#615
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 75.125% when pulling 11055d1 on EdgeCaseBerg:Metrics-Prefix into 39aa95e on brettwooldridge:dev.

@jnehlmeier
Copy link

Seems to look good.

With Metrics 4.x in the future HikariConfig would also need an API to provide metric tags because in Metrics 4.x (currently their master branch) their metrics name will be represented by an instance of MetricName (https://github.com/dropwizard/metrics/blob/master/metrics-core/src/main/java/io/dropwizard/metrics/MetricName.java) which is immutable. This class supports attaching custom tags to a metric so that metric names do not have to be relatively long just because tags are encoded directly in the metric name like I described in my issue.

So with Metrics 4.x you could do:

Map<String, String> tags = ....;
tags.put("customer", "example.com");
tags.put("host", "cluster-host-01");
tags.put("jvm-instance", "1");

// with "metricPrefix" just being, e.g. "connection-pool" or "hikaricp".
registry.register(new MetricName(metricPrefix + ".activeConnections", tags), someGauge);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants