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

Add custom tags to MicrometerCapability #2305

Merged

Conversation

BartekBanachowicz
Copy link
Contributor

This pull request adds new constructors to MicrometerCapability, so the clients are now able to add custom tags to MeterRegistry from the builder level.

Two new constructors were added. The first one can take a List<Tags>:

public MicrometerCapability(MeterRegistry meterRegistry, List<Tag> tagsList)

The second one can take a Map<String, String> of Keys and Values:

public MicrometerCapability(MeterRegistry meterRegistry, Map<String, String> tagsMap)

Closes #1456

Copy link
Member

@velo velo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good, any chance to replicate this on dropwizard metrics too?


public MicrometerCapability(MeterRegistry meterRegistry, Map<String, String> tagsMap) {
List<Tag> tagsList = mapTags(tagsMap);
meterRegistry.config().commonTags(tagsList);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this would be testable under our current tests?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BartekBanachowicz what about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, the answer from my side is no. Not because it isn't doable, but because I don't know how to achieve it yet. I'm still digging through existing test classes

@BartekBanachowicz
Copy link
Contributor Author

Look good, any chance to replicate this on dropwizard metrics too?

Of course, but I'd prefer to do it in a separate issue

@velo velo merged commit 1e03ac3 into OpenFeign:master Jan 25, 2024
3 checks passed
velo added a commit that referenced this pull request Oct 7, 2024
* #1456 | Add custom tags to MicrometerCapability

* Update MicrometerCapability.java license year

---------

Co-authored-by: Marvin <velo@users.noreply.github.com>
velo added a commit that referenced this pull request Oct 8, 2024
* #1456 | Add custom tags to MicrometerCapability

* Update MicrometerCapability.java license year

---------

Co-authored-by: Marvin <velo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Support custom tags for Micrometer
2 participants