micrometer-metrics / micrometer

An application observability facade for the most popular observability tools. Think SLF4J, but for observability.
https://micrometer.io
Apache License 2.0
4.41k stars 970 forks source link

Improve MeterTag to allow extracting multiple keys/value tags #4081

Open gwelican opened 10 months ago

gwelican commented 10 months ago

Please describe the feature request. The '@MeterTag' feature lets you add dynamic tags, taking a value from a parameter. Currently, you can only extract one key/value pair because annotation can't be repeated and I did not see a way to define a list of key/value pairs. It would be helpful to allow for extracting multiple key-value pairs.

Rationale For APIs with numerous fields, it would be beneficial to create multiple tags for improved filtering.

Additional context

marcingrzejszczak commented 6 months ago

Maybe I'm missing sth, can you provide an example of what doesn't work currently?

gwelican commented 6 months ago

hey @marcingrzejszczak, thanks for taking a look. Suppose we have the following code:

public ChargeResponse chargeInstrument(ChargeInstrumentRequest chargeInstrumentRequest) {
      }

public class ChargeInstrumentRequest {
 String currency; // EUR, USD etc
 String instrumentType; // CARD, PAYPAL etc
}

I can use @MeterTag to extract 1 field:

      @MeterTag(key = "instrument_type", expression = "instrumentType")

If I also want to use the currency as a tag, I don't see a way to do it with @MeterTag, I cannot repeat @MeterTag and @MeterTag does not accept a list of keyvaluepairs, I could implement my custom extractor, but would be nice to use spel with @MeterTag.

I am still on spring boot 2.x, planning to upgrade soon, my apologies if this was solved in 3.x, but from the micrometer documentation, it seemed like this is still going to be a problem in 3.x

marcingrzejszczak commented 6 months ago

Do I understand correctly that you would like to use a single method parameter as 2 tags? In other words annotate the currency parameter twice with different values?

gwelican commented 6 months ago

Correct, I'd like to do

public ChargeResponse chargeInstrument(@MeterTag(key = "instrument_type", expression = "instrumentType") @MeterTag(key = "currency", expression = "currency") ChargeInstrumentRequest chargeInstrumentRequest) {
}

Or if the annotation cannot be repeated(it is just an example). Providing a key-value pair is probably more readable, where the key is the metric key and the value is the spel that resolves the value from the input parameter.

smaxx commented 3 months ago

it seems that the only missing thing is the declaration of @Repeatable(MeterTags.class) where MeterTags is just:

@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.PARAMETER})
@Documented
public @interface MeterTags {
    MeterTag[] value();
}
marcingrzejszczak commented 2 months ago

Care for a PR?

smaxx commented 2 months ago

Sure https://github.com/micrometer-metrics/micrometer/pull/5055

shakuzen commented 2 weeks ago

Unfortunately, I had to revert this change as it broke the build and we need to get 1.14.0-M1 released. See https://github.com/micrometer-metrics/micrometer/pull/5055#issuecomment-2216847415

smaxx commented 1 week ago

Unfortunately, I had to revert this change as it broke the build and we need to get 1.14.0-M1 released. See #5055 (comment)

Please see the new one: https://github.com/micrometer-metrics/micrometer/pull/5292 Some additional comments: Merging of the parameter annotations works now in a bit different way, earlier it was based on parameter index, now we support all of the annotations taken from target class and base interface methods, and the actual deduplication happens not based on a parameter index, but on a metertag key instead. So you can now have multiple annotations on different layers, they will be intelligently deduped, more specific keys will take precedence in case if we have same key in multiple declarations(class, interface)