Broken graphite metrics names with tags

[davide-baldo]

Graphite tags always generate broken names due to DropWizard behavior.

From DropWizard, GraphiteReporter [1]:
graphite.send(prefix(name, type.getCode()), format(value), timestamp);
where
name="my.metric;tag=value"
type.getCode() = "p99"

final string sent to graphite: "my.metric;tag=valuep99"

From graphite logs
08/05/2020 08:21:00 :: [tagdb] Tagging component.api.setStatus;host=mail.example.com.mean, component.api.sendTextMessage;host=mail.example.com.p95, component.api.join_meeting;host=mail.example.com.m1_rate, component.api.getConversations;host=mail.example.com.mean

Unless we patch DropWizard side i can't see how this code may work

[1] https://github.com/dropwizard/metrics/blob/v4.1.7/metrics-graphite/src/main/java/com/codahale/metrics/graphite/GraphiteReporter.java#L351

[davide-baldo]

@fitzoh @shakuzen
What do you think it's the best approach?

[fitzoh]

Yeah, saw your comment yesterday, should be able to dig in this weekend.
Hoping I can find an easy fix, if not the easy first step is probably updating defaults

[fitzoh]

I opened a PR (dropwizard/metrics#1576) to dropwizard metrics that should allow us to work around this.
No idea what kind of time frame we will be looking at for that to be merged or for a new release to be cut.

I agree with @workless that there isn't a clean way to fix this without modifying the dropwizard code.

The options I can think of:

• Wait for the dropwizard metrics maintainers to merge/cut a new release, then modify on the micrometer side
• fork/publish a modified version of the graphite reporter and work off that
• vendor the modified version of the graphite reporter in this repo and use that until an official release is out
• revert the default and/or add a note and/or remove the feature from micrometer until the dropwizard metrics release is available

Either way, the Spring Boot release team should probably be made aware, as I believe this feature is present in v2.3.0.RC1 (cc @snicoll)

[jkschneider]

@fitzoh It may be the reason we need to cut ties with Dropwizard as a passthrough for Graphite. We had a similar situation with the Ganglia registry a while back.

[fitzoh]

For what it's worth, the upstream PR was merged already. I opened a draft PR (#2083) with a failing test to cover this scenario. I'm planning on adding a fix to that branch which pulls in a SNAPSHOT version of dropwizard metrics, then dropping the SNAPSHOT once a new dropwizard version is out.

[fitzoh]

PR has been updated to work with the Dropwizard SNAPSHOT, should be good to go once a release is cut.