Skip to content

Conversation

@izeye
Copy link
Contributor

@izeye izeye commented Apr 28, 2022

This PR polishes Micrometer tracing changes a bit.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 28, 2022
Copy link
Member

@wilkinsona wilkinsona left a comment

Choose a reason for hiding this comment

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

Thanks very much for the PR, @izeye. I've left a couple of comments for your consideration when you have time.

assertThat(context).hasBean("customTracer");
assertThat(context).hasSingleBean(Tracer.class);
});
this.contextRunner.withUserConfiguration(CustomConfiguration.class).run((context) -> {
Copy link
Member

Choose a reason for hiding this comment

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

I think we need OpenTelemetryConfiguration here so that @ConditionalOnBean(OpenTelemetry.class) on OpenTelemetryConfigurations.TracerConfiguration.otelTracer(OpenTelemetry) is satisfied and the test is then checking the @ConditionalOnMissingBean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wilkinsona Oops, I missed it. I reverted this change.

}

@Test
void shouldNotSupplyRestTemplateSenderIfNoBuilderIsAvailable() {
Copy link
Member

Choose a reason for hiding this comment

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

Just to double-check my understanding, I assume you have removed this test as it duplicates shouldSupplyBeans()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wilkinsona Yes, it seems to be a duplicate of shouldSupplyBeans().

Copy link
Member

Choose a reason for hiding this comment

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

👍

@izeye izeye force-pushed the polish-20220428 branch from 3bea0e6 to 258db71 Compare April 29, 2022 09:19
@wilkinsona wilkinsona added this to the 3.0.x milestone Apr 29, 2022
@wilkinsona wilkinsona added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 29, 2022
wilkinsona pushed a commit that referenced this pull request May 3, 2022
@wilkinsona wilkinsona closed this in 11efe24 May 3, 2022
@wilkinsona wilkinsona modified the milestones: 3.0.x, 3.0.0-M3 May 3, 2022
@izeye izeye deleted the polish-20220428 branch May 3, 2022 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: task A general task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants