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

Change B3 extraction format to single #36061

Closed
wants to merge 2 commits into from
Closed

Change B3 extraction format to single #36061

wants to merge 2 commits into from

Conversation

KitBolourchi
Copy link
Contributor

Link to issue ticket: #35674

Change B3 propagation to B3Propagation.Format.SINGLE

@KitBolourchi KitBolourchi changed the title ISSUE: 35611 - Be extraction ISSUE: 35611 - B3 extraction Jun 24, 2023
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 24, 2023
@mhalbritter mhalbritter changed the title ISSUE: 35611 - B3 extraction Change B3 extraction format to SINGLE Jun 26, 2023
@mhalbritter mhalbritter added type: enhancement A general enhancement theme: observability Issues related to observability and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 26, 2023
@mhalbritter mhalbritter added this to the 3.2.x milestone Jun 26, 2023
This was referenced Jun 26, 2023
Closed
Closed
@mhalbritter mhalbritter added the status: waiting-for-feedback We need additional information before we can continue label Jun 27, 2023
jonatan-ivanov
jonatan-ivanov requested changes Jul 12, 2023
View reviewed changes
...org/springframework/boot/actuate/autoconfigure/tracing/CompositePropagationFactoryTests.java Outdated
@@ -86,6 +93,24 @@ void inject() {
assertThat(request).containsOnly(entry("a", "a-value"), entry("b", "b-value"));
}

@Test
void thisIsATest() {
Copy link
Member

Choose a reason for hiding this comment

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

Could you please name it based on what it does?
E.g.: shouldUseB3SingleWithParentWhenPropagationTypeIsB3

...org/springframework/boot/actuate/autoconfigure/tracing/CompositePropagationFactoryTests.java Outdated Show resolved Hide resolved
...org/springframework/boot/actuate/autoconfigure/tracing/CompositePropagationFactoryTests.java Outdated Show resolved Hide resolved
...org/springframework/boot/actuate/autoconfigure/tracing/CompositePropagationFactoryTests.java Outdated Show resolved Hide resolved
@jonatan-ivanov
Copy link
Member

I rebased your changes on main, squashed your commits together and added a test: main...jonatan-ivanov:spring-boot:b3-single
What do you think?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 13, 2023
@KitBolourchi
Copy link
Contributor Author

I see, yes this looks good. That makes a lot of sense and clearly outlines the mistakes I made. Would you propose I close this PR down and continue with your changes or I rectify and fix my tests for this PR?

@jonatan-ivanov
Copy link
Member

I don't think there is a need for another PR, you can fetch my branch and reset yours to my last commit (you will need to force push).

@KitBolourchi KitBolourchi force-pushed the 35611_B3_Extraction branch from 3464fd8 to 7df07d7 Compare July 13, 2023 20:36
@KitBolourchi
Copy link
Contributor Author

I don't think there is a need for another PR, you can fetch my branch and reset yours to my last commit (you will need to force push).

Should be all done now. @jonatan-ivanov Please check that all is aligned and as expected. I appreciate the help, I learned a lot with this PR :)

@mhalbritter mhalbritter removed the status: feedback-provided Feedback has been provided label Jul 18, 2023
@mhalbritter mhalbritter self-assigned this Jul 19, 2023
@mhalbritter mhalbritter changed the title Change B3 extraction format to SINGLE Change B3 extraction format to single Jul 19, 2023
mhalbritter pushed a commit that referenced this pull request Jul 19, 2023
@KitBolourchi @mhalbritter
Change B3 extraction format to single
abebc39 
See gh-36061
@mhalbritter mhalbritter modified the milestones: 3.2.x, 3.2.0-M1 Jul 19, 2023
@mhalbritter
Copy link
Contributor

Thank you all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhalbritter mhalbritter mhalbritter left review comments

@jonatan-ivanov jonatan-ivanov jonatan-ivanov approved these changes

Assignees

@mhalbritter mhalbritter

Labels
theme: observability Issues related to observability type: enhancement A general enhancement
Projects
None yet
Milestone
3.2.0-M1
Development

Successfully merging this pull request may close these issues.
4 participants
@KitBolourchi @jonatan-ivanov @mhalbritter @spring-projects-issues