Make ZipkinHttpClientSender the default BytesMessageSender

[mhalbritter]

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.

[wickdynex]

Follow-Up Questions

Hi @mhalbritter,

I have a few follow-up questions regarding this issue:

1. Develop branch: u didn't mentioned which branch would apply this modification, whether main or 3.2.x or someone else?🤔
2. Proposed Steps: to switch the UrlConnectionSenderConfiguration and HttpClientSenderConfiguration, 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.🥳

[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 #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.

[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?

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

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

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 like 3.4.x branch, how could you apply the PR in the new branch? By cherry-pick or 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.

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

[mhalbritter]

2. Proposed Steps: to switch the UrlConnectionSenderConfiguration and HttpClientSenderConfiguration, I supposed only to switch the import order, after switching, it may look like this:

Yes, this works.

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?

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.

[wickdynex]

2. Proposed Steps: to switch the UrlConnectionSenderConfiguration and HttpClientSenderConfiguration, I supposed only to switch the import order, after switching, it may look like this:

Yes, this works.

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?

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.

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❤️.

[mhalbritter]

In the scope of this issue, you should just reorder the configurations, like you said in #42589 (comment). 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.