extraTags is array but key->value

[TheHett]

For Counter/Timer annotation argument extraTags defined as array of string, but requires even items becuase it is key->value pair.
It's not obvious.
I spet half day before until I saw the documentation.
Then I remembered that I had already encountered this a couple of years ago.

Maybe it's worth adding a warning to log?

[jonatan-ivanov]

Thanks for the issue!
My brain is definitely spoiled here and I cannot unlearn the knowledge that tags are key-value pairs so your input on this would be definitely helpful.

Could you please tell us:

• What you tried to do before you looked at the docs?
• What was your first impression about the usage of extraTags, what did you think it does?
• Why you spent half a day before looking at the docs?

I think we could improve the javadocs and add this fact to even more places but right now you can see this if you look into the docs:

micrometer/micrometer-core/src/main/java/io/micrometer/core/annotation/Counted.java

Lines 55 to 61 in c39bda3

	/**
	* List of key-value pair arguments to supply the Counter as extra tags.
	* @return key-value pair of tags
	* @see io.micrometer.core.instrument.Counter.Builder#tags(String...)
	* @since 1.4.0
	*/
	String[] extraTags() default {};

micrometer/micrometer-core/src/main/java/io/micrometer/core/instrument/Counter.java

Lines 77 to 84 in c39bda3

	/**
	* @param tags Must be an even number of arguments representing key/value pairs of
	* tags.
	* @return The counter builder with added tags.
	*/
	public Builder tags(String... tags) {
	return tags(Tags.of(tags));
	}

micrometer/micrometer-core/src/main/java/io/micrometer/core/annotation/Timed.java

Lines 46 to 51 in c39bda3

	/**
	* List of key-value pair arguments to supply the Timer as extra tags.
	* @return key-value pair of tags
	* @see io.micrometer.core.instrument.Timer.Builder#tags(String...)
	*/
	String[] extraTags() default {};

micrometer/micrometer-core/src/main/java/io/micrometer/core/instrument/AbstractTimerBuilder.java

Lines 54 to 61 in c39bda3

	/**
	* @param tags Must be an even number of arguments representing key/value pairs of
	* tags.
	* @return This builder.
	*/
	public B tags(String... tags) {
	return tags(Tags.of(tags));
	}

I think we could add a sentence that the array must have an even number of elements in Counted.java and Timed.java as well but you you should get an exception if you don't:

micrometer/micrometer-core/src/main/java/io/micrometer/core/instrument/Tags.java

Lines 362 to 364 in c39bda3

	if (keyValues.length % 2 == 1) {
	throw new IllegalArgumentException("size must be even, it is a set of key=value pairs");
	}

Do you want to open a PR to improve the docs of Counted.java and Timed.java?

[TheHett]

Hi!

What you tried to do before you looked at the docs?

First I tried to use single value for tags. It not lead to error or any log, but metric was not presented in actuator.

    @Timed("batcher", extraTags = ["write"])

What was your first impression about the usage of extraTags, what did you think it does?

I have some work experience with prometheus and I know what the end result looks like, but I just got stuck :)

Why you spent half a day before looking at the docs?

Half a day is not literally. I tried searching cause in another mechanisms. First I thougt about incorrect using Bean (our code call proxied bean from non-proxied and I spend time to remade it, but it had no effect. Then I thought about incorrect aspectjwaver install and that this problem may related with spring upgrading. I did not focus on this annotation first.

I think it would be a good idea to add a log or fail startup when incorrect items count provided for extraTags.

[TheHett]

I think we could add a sentence that the array must have an even number of elements in Counted.java and Timed.java as well but you you should get an exception if you don't

Hmm, no. It's not work, no any exceptions.

    runtimeOnly("io.micrometer:micrometer-registry-prometheus:1.13.4")

[jonatan-ivanov]

First I tried to use single value for tags. It not lead to error or any log, but metric was not presented in actuator.

   @Timed("batcher", extraTags = ["write"])

What was your expectation what should have happened in this case?

Hmm, no. It's not work, no any exceptions.

Do you have a minimal sample project to reproduce this issue? Or could you please check with a debugger where the exception is swallowed so that we can fix this? In usual setups TimedAspect should call Tags.of(String...) which should throw an exception but I'm not sure if you use TimedAspect or something else wraps your annotated methods.

[TheHett]

What was your expectation what should have happened in this case?

I didn't think about it :)

In usual setups TimedAspect should call Tags.of(String...) which should throw an exception but I'm not sure if you use TimedAspect or something else wraps your annotated methods.

Yes, I using Timed/Counted aspects. And its has different behavior:

• Counted throws this exception IllegalArgumentException("size must be even, it is a set of key=value pairs");
• Timed has no an exception.

I debugged it and found ignoring:

https://github.com/micrometer-metrics/micrometer/blob/main/micrometer-core/src/main/java/io/micrometer/core/aop/TimedAspect.java#L275

  private void record(ProceedingJoinPoint pjp, Timed timed, String metricName, Timer.Sample sample,
            String exceptionClass) {
        try {
            sample.stop(recordBuilder(pjp, timed, metricName, exceptionClass).register(registry));
        }
        catch (Exception e) {
            // ignoring on purpose
        }
    }

Sorry, I can't provide PR becuase I don't know why is it implemented so differently. Maybe it should be globally refactored.