MetricName well-defined toString + valueOf(String)
mbrannstrom opened this issue · 3 comments
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:
- InfluxDB use the syntax (line protocol syntax):
weather,location=us-midwest
- Prometheus use the syntax
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
vs dotted.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 new tagged MetricName should have a well-defined string syntax.
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.
However, without escaping that representation is not parsable.
I'm not sure it's reasonable to expect the MetricName.toString()
output to be machine-parsable anymore. The string serialization format of MetricName
is dependent upon the metrics reporting system, and no longer an intrinsic property of the name itself; Any single serialization format is going to cause problems for someone. toString()
should use a format that is human-parsable, best suited for developers that see it appear in logs or debuggers. dottedName + Map.toString()
is actually very good for that, since Map.toString()
something that developers are already used to parsing visually.
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 toString
can be parsed by the corresponding valueOf
/parseXxx
method. E.g. Integer.toString
vs Integer.valueOf
and File.toString
= File.getPath
vs new File
.
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.
Perhaps this would be better solved with an interface that reporters can leverage:
public interface MetricNameFormat
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.