search

openrewrite / rewrite-spring

OpenRewrite recipes for Spring projects.
Apache License 2.0
237 stars 64 forks source link

Add property change from management.metrics.export.atlas.aggrConfig to management.atlas.metrics.export.aggrConfig in UpgradeSpringBoot_3_0 recipe #517

Closed jjacobs44 closed 2 months ago

jjacobs44 commented 2 months ago

What's changed?

Added property change from management.metrics.export.atlas.aggrConfig to management.atlas.metrics.export.aggrConfig in UpgradeSpringBoot_3_0

What's your motivation?

Just noticed from personal experience this property change was missing in the recipe 🙂

timtebeek commented 2 months ago

Thanks for reporting that here! There's an interesting bit to this in that we generate those properties from meta data that the folks from Spring themselves make available in their starter jars, meaning it's probably missing there as well. https://github.com/openrewrite/rewrite-spring/blob/006b595420b62765595b7acca75f15bb05bd4c8f/.github/workflows/properties.yml#L22-L26

Ideally this is fixed upstream such that it's also shown in IDEs when old properties are still used; the place for that change would be here: https://github.com/spring-projects/spring-boot/blob/f6d8111a1a6e2da9b7743547f4d2189443caa503/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json#L463-L468

Until then we can include this property with our recipes. I'd opt to move this addition to src/main/resources/META-INF/rewrite/spring-boot-30.yml though, such that it's not lost next time we generate the migration properties.

jjacobs44 commented 2 months ago

I'll get a PR opened up in Spring boot itself, and will remove these changes when that's been released. Thank you for all the helpful context! Also, happy to move things around if anything about this setup is not what you were suggesting.

timtebeek commented 2 months ago

Awesome, thanks a lot for picking this up and also updating Spring Boot!

philwebb commented 1 month ago

It turns out there's no aggrConfig property to migrate (see https://github.com/spring-projects/spring-boot/pull/40968#issuecomment-2145739703) so you might want to revert this one.