-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
MetricName well-defined toString + valueOf(String) #2167
Comments
With the addition of structured tagging, it should be configurable, not well defined, since it needs to match and work with the tags people already have embedded in their 4.x metric names. I might have 3 different reporters configured, and each one might need tags to be surfaced in a different format. More specific to my situation, for a jump to 5.X to be reasonable, I would need the JSON serialization format to match the tag format I have currently embedded in the 4.X names, because it's not otherwise feasible to upgrade several thousand applications and our metrics processing pipeline to use a different format all at once.
I'm not sure it's reasonable to expect the Perhaps this would be better solved with an interface that reporters can leverage: public interface MetricNameFormat {
static MetricNameFormat INFLUXDB = new InfluxDBNameFormat();
static MetricNameFormat PROMETHUS = new PromethusNameFormat();
String serialize(MetricName metricName);
} |
This issue is stale because it has been open 180 days with no activity. Remove the "stale" label or comment or this will be closed in 14 days. |
@baharclerode: I disagree, and you're missing the point here. Any API should have a well defined behaviour. Not a random behaviour that "just happened" by some arbitrary implementation. In the JDK it is common that The InfluxDB line protocol syntax is an example of how to define the MetricName.toString, that happens to be well-defined and simple (and human readable). It is not intended to be a replacement for serialization for a specific metric database.
This is not a good idea. This serialization should be up to the reporter to e.g. InlfuxDB and Prometheus. Not part of the core Metrics API. The core API should be small, and not affected by new reporters. |
To simplify migration from the plain
dotted.name
syntax (in 4.x versions), usually represented as a Java String, the new tagged MetricName should have a well-defined string syntax.For example, dropwizard-metrics-graphite uses a String for its'
prefix
in the configuration.Currently the MetricName have the toString() representation of
dotted.name{tag1=value, tag2=value}
, i.e. dottedName + Map.toString(). However, without escaping that representation is not parsable.As a comparison:
weather,location=us-midwest
bicycle_distance_meters_total{brand="monark",gears="7"}
Discussion:
Should we stick to the existing string representation, or use e.g. the InfluxDB syntax, which is well-defined (explicit escaping rules), slimmer than Prometheus' and more liberate in allowed characters? The InfluxDB line protocol syntax is also backwards compatible when not using tags, e.g.
dotted.name
vsdotted.name,with=tags
Update:
Note: A common "prefix" is to add tags for an application, e.g.
environment=production,region=eu-west
without the "dotted.name" part.What do you think? Should we go for a InfluxDB-inspired syntax?
The text was updated successfully, but these errors were encountered: