Skip to content
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

Allow multiple MeterTag annotations for multiple tags from same target #4081

Closed
gwelican opened this issue Sep 13, 2023 · 9 comments · Fixed by #5055 or #5292
Closed

Allow multiple MeterTag annotations for multiple tags from same target #4081

gwelican opened this issue Sep 13, 2023 · 9 comments · Fixed by #5055 or #5292
Labels
enhancement A general enhancement module: micrometer-core An issue that is related to our core module
Milestone

Comments

@gwelican
Copy link

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
Copy link
Contributor

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

@marcingrzejszczak marcingrzejszczak added the waiting for feedback We need additional information before we can continue label Dec 28, 2023
@gwelican
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

gwelican commented Jan 1, 2024

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.

@marcingrzejszczak marcingrzejszczak added enhancement A general enhancement and removed waiting for feedback We need additional information before we can continue waiting-for-triage labels Jan 4, 2024
@smaxx
Copy link
Contributor

smaxx commented Apr 4, 2024

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
Copy link
Contributor

Care for a PR?

@smaxx
Copy link
Contributor

smaxx commented May 7, 2024

Sure #5055

@jonatan-ivanov jonatan-ivanov added the module: micrometer-core An issue that is related to our core module label May 7, 2024
@jonatan-ivanov jonatan-ivanov added this to the 1.14.0-M1 milestone May 7, 2024
shakuzen pushed a commit that referenced this issue Jul 9, 2024
…arameter (#5055)

Resolves gh-4081

---------

Co-authored-by: Maksym Symonov <maksym.symonov@nordstrom.com>
@shakuzen shakuzen modified the milestones: 1.14.0-M1, 1.14.x Jul 9, 2024
@shakuzen
Copy link
Member

shakuzen commented Jul 9, 2024

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)

@shakuzen shakuzen reopened this Jul 9, 2024
@shakuzen shakuzen modified the milestones: 1.14.x, 1.14.0-M2 Jul 9, 2024
@smaxx
Copy link
Contributor

smaxx commented Jul 9, 2024

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: #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)

@shakuzen shakuzen changed the title Improve MeterTag to allow extracting multiple keys/value tags Allow multiple MeterTag annotations for multiple tags from same target Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement module: micrometer-core An issue that is related to our core module
Projects
None yet
5 participants