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.

Sign up for GitHub

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

Jump to bottom

Support Micrometer @MeterTag #36945

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Support Micrometer @MeterTag #36945

merged 1 commit into from
Nov 28, 2023

Conversation

computerlove
Copy link
Contributor

@computerlove computerlove commented Nov 8, 2023 •
edited
Loading

Micrometer defines a annotation @MeterTag, making it possible to add additional tags for methods annotated with @counted and @timed from method arguments.

@computerlove computerlove marked this pull request as draft November 8, 2023 12:53
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Nov 8, 2023

/cc @brunobat (micrometer), @ebullient (micrometer)

@computerlove computerlove marked this pull request as ready for review November 13, 2023 10:00
@computerlove
Copy link
Contributor Author

I have made a suggestion for how to support @MeterTag.

MeterTag.resolver is supported, the application can define beans imlementing ValueResolver and refere to these classes in MeterTag.resolver=ValueResolverBean.class.

MeterTag.expression is also supported, the application can define a bean imlementing ValueExpressionResolver and it will be used to evaluate the expression.

@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@brunobat
Copy link
Contributor

There is an error in the docs that needs fixing

@computerlove computerlove force-pushed the metertag branch 2 times, most recently from 55c401f to 1374356 Compare November 13, 2023 19:24
@computerlove
Copy link
Contributor Author

It's now fixed.

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 13, 2023 •
edited
Loading

🙈 The PR is closed and the preview is expired.

@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
gsmet
gsmet previously requested changes Nov 14, 2023
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Just a note that this will need a squash before merging. Please dismiss when done.

@computerlove computerlove force-pushed the metertag branch 2 times, most recently from 323e63c to da5d75d Compare November 14, 2023 18:33
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@computerlove
Copy link
Contributor Author

Commits has been squashed.

brunobat
brunobat reviewed Nov 15, 2023
View reviewed changes
Copy link
Contributor

@brunobat brunobat left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@computerlove would you be able to add an integration test to integration-tests/micrometer-prometheus?... Mimicking the usage you have in the docs would be perfect.

@computerlove
Copy link
Contributor Author

@computerlove would you be able to add an integration test to integration-tests/micrometer-prometheus?... Mimicking the usage you have in the docs would be perfect.

Yes, will do!

@computerlove computerlove force-pushed the metertag branch 2 times, most recently from 587fa89 to 49cff9c Compare November 16, 2023 09:47
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@computerlove
Copy link
Contributor Author

I have fixed the comments for documentation and imports.

@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Nov 27, 2023

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

@ebullient
Copy link
Member

@gsmet .. I think we're good to go.

@gsmet gsmet dismissed their stale review November 28, 2023 15:20

Adressed.

@gsmet gsmet merged commit 88e7549 into quarkusio:main Nov 28, 2023
31 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Nov 28, 2023
@computerlove computerlove deleted the metertag branch November 30, 2023 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebullient ebullient ebullient approved these changes

@brunobat brunobat Awaiting requested review from brunobat

@gsmet gsmet Awaiting requested review from gsmet

Assignees
No one assigned
Labels
area/agroal area/arc Issue related to ARC (dependency injection) area/cache area/config area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/docstyle issues related for manual docstyle review area/documentation area/google-cloud-functions area/gradle Gradle area/graphics area/graphql area/grpc gRPC area/health area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/jaeger area/kafka area/kafka-streams area/keycloak area/kubernetes area/logging area/maven area/metrics area/mongodb area/narayana Transactions / Narayana area/oidc area/persistence OBSOLETE, DO NOT USE area/platform Issues related to definition and interaction with Quarkus Platform area/qute The template engine area/reactive-messaging area/rest area/resteasy-classic area/security area/smallrye area/testing area/tracing area/vertx release/noteworthy-feature
Projects
Quarkus Documentation
Status: Done
Milestone
3.7.0.CR1
Development

Successfully merging this pull request may close these issues.
4 participants
@computerlove @brunobat @ebullient @gsmet