MetricsUtil.formatString contains unnecessary code?

[dlmarion]

MetricsUtil in the elasticity branch contains a formatString method. In this method it has the following code to handle camel case values in the metric names:

    // Handle camelCase notation
    Matcher matcher = camelCasePattern.matcher(name);
    StringBuilder output = new StringBuilder(name);
    int insertCount = 0;
    while (matcher.find()) {
      // Pattern matches at a lowercase letter, but the insert is at the second position.
      output.insert(matcher.start() + 1 + insertCount, ".");
      // The correct index position will shift as inserts occur.
      insertCount++;
    }
    name = output.toString();
    return name.toLowerCase();

If you notice at the end of the method, it returns name.toLowerCase(). I think this should handle the camel case situation and that code can just be removed.

[ddanielr]

I don't think this is a bug.

The goal of the format string method is to replace any intended delimiters with the dot "." delimiter that is used by micrometer.

Micrometer will then transform dot delimiters to the delimiter used by the metric producer

In the camelCase example, this code is changing a string like compactorQueue to compactor.queue.

So for Prometheus, compactor.queue would become compactor_queue.

If the code just used toLowerCase, than a user would have to search for a tag value of compactorqueue which has low readability.

[ddanielr]

I could see how this method might be a bit unclear.

To make this clearer, the formatString method could be renamed to something like replaceDelimiters

[dlmarion]

In the camelCase example, this code is changing a string like compactorQueue to compactor.queue.

Ok, I didn't get that from looking at that code. I think we should keep this issue open and the change will be for someone to add a comment to that effect for clarity.

[ddanielr]

Comments have been updated in #4189