Right now, org.springframework.boot.actuate.autoconfigure.tracing.zipkin.ZipkinConfigurations.SenderConfiguration imports UrlConnectionSenderConfiguration before HttpClientSenderConfiguration. We should switch that around. Also we should think about removing support for URLConnectionSender altogether.
The ZipkinRestTemplateSender and ZipkinWebClientSender will be removed in Boot 3.5.0, too.
Comment From: wickdynex
Follow-Up Questions
Hi @mhalbritter,
I have a few follow-up questions regarding this issue:
- Develop branch: u didn't mentioned which branch would apply this modification, whether
mainor3.2.xor someone else?🤔 - Proposed Steps: to switch the
UrlConnectionSenderConfigurationandHttpClientSenderConfiguration, I supposed only to switch the import order, after switching, it may look like this:
@Configuration(proxyBeanMethods = false)
@Import({
HttpClientSenderConfiguration.class,
WebClientSenderConfiguration.class,
RestTemplateSenderConfiguration.class,
UrlConnectionSenderConfiguration.class
})
does it works? Need I create a branch and apply this? What's your opinion?🥺
3. Future Plans: have you also the team haved a meeting about whether removing this URLConnectionSender @Bean? At the same time, should I also remove the code which depending on these components?🤨
4. Assigned Task: have u already assigned this task to a person but not mentioned in this #42589 issue? 🥸
There is one thing I must clarify, I'm a green hand in spring-boot also java, did't participate in any huge program. If there is any mistake I've made, plz pardon me.😭❤️ I'm looking forward to you reply, thanks for answering my question.🥳
Comment From: philwebb
Develop branch: u didn't mentioned which branch would apply this modification, whether main or 3.2.x or someone else?🤔
The milestone target (top right of the issue) is set to 3.5.x for this one. This means we want to do it for the 3.5 release, which will be on main once we've branched. We usually branch about a month after the 3.4.0 release.
Assigned Task: have u already assigned this task to a person but not mentioned in this https://github.com/spring-projects/spring-boot/issues/42589 issue? 🥸
No, the issue is currently unassigned. We change "Assignees" when someone actively starts working on an issue.
I'll leave the other questions for @mhalbritter to answer.
Comment From: wickdynex
The milestone target (top right of the issue) is set to 3.5.x for this one. This means we want to do it for the 3.5 release, which will be on main once we've branched. We usually branch about a month after the 3.4.0 release.
It means I can't fix this issue before 3.5.x branched? Am I misunderstanding your meaning?🤔 So could you plz explain the meaning of opening this issue to me? Maybe I need to learn the GitHub basic usage of issues🥺, thank u.
No, the issue is currently unassigned. We change "Assignees" when someone actively starts working on an issue.
But now we didn't branch the 3.4.x, so it means this issue can't assign to anyone?
Comment From: philwebb
It means I can't fix this issue before 3.5.x branched? Am I misunderstanding your meaning?🤔 So could you plz explain the meaning of opening this issue to me?
You're welcome to take a look at the issue and fix it on main, but we won't be able to really review or merge it until we create a 3.4.x branch.
But now we didn't branch the 3.4.x, so it means this issue can't assign to anyone?
No, the issue can be assigned at any time. You're also welcome to submit a pull-request on any unassigned issue. Our wrokflow is that we assign an issue when we know somebody is actively working on it. This aims to prevent duplicated effort.
Comment From: wickdynex
It means I can't fix this issue before 3.5.x branched? Am I misunderstanding your meaning?🤔 So could you plz explain the meaning of opening this issue to me?
You're welcome to take a look at the issue and fix it on
main, but we won't be able to really review or merge it until we create a3.4.xbranch.But now we didn't branch the 3.4.x, so it means this issue can't assign to anyone?
No, the issue can be assigned at any time. You're also welcome to submit a pull-request on any unassigned issue. Our wrokflow is that we assign an issue when we know somebody is actively working on it. This aims to prevent duplicated effort.
Appreciate for your reply😇. There's another question:
if I open a PR to fix the bug on
main branch, after you and your team create a new branch like3.4.x branch, how could you apply the PR in the new branch? Bycherry-pickor any operate else🤨?
And there are also the rest of the questions in my previous comment, Im looking forward to the reply😃. Anyway, thank u again.
Comment From: philwebb
how could you apply the PR in the new branch? By cherry-pick or any operate else
We can rebase the PR. That's a usual part of our process.
Comment From: mhalbritter
2. Proposed Steps: to switch the
UrlConnectionSenderConfigurationandHttpClientSenderConfiguration, I supposed only to switch the import order, after switching, it may look like this:
Yes, this works.
- Future Plans: have you also the team haved a meeting about whether removing this URLConnectionSender @bean? At the same time, should I also remove the code which depending on these components?
I'd remove WebClientSenderConfiguration, RestTemplateSenderConfiguration and UrlConnectionSenderConfiguration altogether. This only leaves HttpClientSenderConfiguration, which makes the ZipkinHttpClientSender the default Zipkin sender. But we should do this in a separate issue.
@wickdynex Do you want to work on that issue? If so, i'd assign it to you.
Comment From: wickdynex
- Proposed Steps: to switch the
UrlConnectionSenderConfigurationandHttpClientSenderConfiguration, I supposed only to switch the import order, after switching, it may look like this:Yes, this works.
- Future Plans: have you also the team haved a meeting about whether removing this URLConnectionSender @bean? At the same time, should I also remove the code which depending on these components?
I'd remove
WebClientSenderConfiguration,RestTemplateSenderConfigurationandUrlConnectionSenderConfigurationaltogether. This only leavesHttpClientSenderConfiguration, which makes theZipkinHttpClientSenderthe default Zipkin sender. But we should do this in a separate issue.@wickdynex Do you want to work on that issue? If so, i'd assign it to you.
It's my pleasure to have a chance to work on this issue, I will try my best to achieve this target. But I'm new in this project🥺, do you have some advice or idea for deleting these configurations? You can have a try to assign this issue to my, but in case to avoid that I mess it up, I will ask and also comments a lot. Pardon me🥰.
Steps below, and you can point out the mistakes :
1. remove all the configuration class in ZipkinConfigurations.
2. locate the position where use the @Bean like urlConnectionSender, and delete or replace it to @Bean httpClientSender.
3. use unit test to check if it works, if needed, I need to write more test cases.
4. update the document and the comment in the modified files.
Am I right? Looking for your reply and modifications❤️.
Comment From: mhalbritter
In the scope of this issue, you should just reorder the configurations, like you said in https://github.com/spring-projects/spring-boot/issues/42589#issuecomment-2450330858. And I guess some tests needs to be adapted.
For the removal of ZipkinWebClientSender and ZipkinRestTemplateSender I've created #43047.
For the removal of URLConnectionSender I've created #43048.
Comment From: mhalbritter
Closing in favor of #43085.