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

Micrometer - Switch to @ConfigMapping #45820

Merged
merged 1 commit into from
Feb 5, 2025
Merged

Micrometer - Switch to @ConfigMapping #45820

merged 1 commit into from
Feb 5, 2025
+214 −392

Conversation

mcruzdev
Copy link
Contributor

@mcruzdev mcruzdev commented Jan 23, 2025 •
edited by gsmet
Loading

Part of #45446

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Jan 23, 2025

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

gsmet
gsmet reviewed Jan 31, 2025
View reviewed changes
...ter/runtime/src/main/java/io/quarkus/micrometer/runtime/config/runtime/HttpServerConfig.java Outdated
Comment on lines 73 to 80
default void mergeDeprecatedConfig(VertxConfig config) {
// if (!ignorePatterns().isPresent()) {
// ignorePatterns = config.ignorePatterns;
// }
// if (!matchPatterns().isPresent()) {
// matchPatterns = config.matchPatterns;
// }
}
Copy link
Member

Choose a reason for hiding this comment

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

Hey @mcruzdev , what's the status of this one? If this is the only problem remaining, I can probably sort it out and we could get it merged. Let me know!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @gsmet, feel free to change it

@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 4, 2025 •
edited
Loading

🎊 PR Preview db77322 has been successfully built and deployed to https://quarkus-pr-main-45820-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

@gsmet
Copy link
Member

gsmet commented Feb 4, 2025

@mcruzdev I will have a look tomorrow.

@gsmet gsmet force-pushed the config-micrometer branch from 939e979 to 00b423b Compare February 4, 2025 22:41
@gsmet gsmet changed the title Start the conversion to @ConfigMapping (micrometer) Micrometer - Switch to @ConfigMapping Feb 4, 2025
@gsmet
Copy link
Member

gsmet commented Feb 4, 2025

@mcruzdev I made good progress, it was indeed quite complex.

That being said, we cannot merge as we have an IT depending on a Quarkiverse extension that consumes this config.

https://github.com/quarkusio/quarkus/blob/main/integration-tests/observability-lgtm/pom.xml#L26-L30

This needs fixing before we can think about merging this, unfortunately.

/cc @brunobat @alesj @radcortez

brunobat
brunobat reviewed Feb 5, 2025
View reviewed changes
Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

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

Nice!

...ter/runtime/src/main/java/io/quarkus/micrometer/runtime/config/runtime/HttpServerConfig.java Show resolved Hide resolved
@brunobat
Copy link
Contributor

brunobat commented Feb 5, 2025

@gsmet created this: #46095

@gsmet gsmet force-pushed the config-micrometer branch from 00b423b to 58b40e7 Compare February 5, 2025 14:39
@gsmet gsmet marked this pull request as ready for review February 5, 2025 14:39
@gsmet
Copy link
Member

gsmet commented Feb 5, 2025

I rebased, it should be ready for review.

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Feb 5, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 58b40e7.

✅ 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.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Feb 5, 2025 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 58b40e7.

✅ 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.

You can consult the Develocity build scans.

@gastaldi gastaldi merged commit 52d6d44 into quarkusio:main Feb 5, 2025
44 of 46 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.19 - main milestone Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brunobat brunobat brunobat left review comments

@gsmet gsmet gsmet left review comments

@radcortez radcortez radcortez approved these changes

Assignees
No one assigned
Labels
area/metrics
Projects
None yet
Milestone
3.19 - main
Development

Successfully merging this pull request may close these issues.
5 participants
@mcruzdev @gsmet @brunobat @radcortez @gastaldi