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

Inject ObjectProvider into BeanPostProcessors to avoid premature initialization of OpenTelemetry #3171

Merged
merged 1 commit into from
Jun 3, 2021
Merged

Inject ObjectProvider into BeanPostProcessors to avoid premature initialization of OpenTelemetry #3171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ...entation/spring/autoconfigure/httpclients/resttemplate/RestTemplateAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.spring.autoconfigure.httpclients.HttpClientsProperties;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
Expand All @@ -30,7 +31,7 @@ public class RestTemplateAutoConfiguration {

@Bean
public RestTemplateBeanPostProcessor otelRestTemplateBeanPostProcessor(
OpenTelemetry openTelemetry) {
return new RestTemplateBeanPostProcessor(openTelemetry);
ObjectProvider<OpenTelemetry> openTelemetryProvider) {
return new RestTemplateBeanPostProcessor(openTelemetryProvider);
}
}
15 changes: 10 additions & 5 deletions ...entation/spring/autoconfigure/httpclients/resttemplate/RestTemplateBeanPostProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.spring.httpclients.RestTemplateInterceptor;
import java.util.List;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.http.client.ClientHttpRequestInterceptor;
import org.springframework.web.client.RestTemplate;

final class RestTemplateBeanPostProcessor implements BeanPostProcessor {
private final OpenTelemetry openTelemetry;
private final ObjectProvider<OpenTelemetry> openTelemetryProvider;

RestTemplateBeanPostProcessor(OpenTelemetry openTelemetry) {
this.openTelemetry = openTelemetry;
RestTemplateBeanPostProcessor(ObjectProvider<OpenTelemetry> openTelemetryProvider) {
this.openTelemetryProvider = openTelemetryProvider;
}

@Override
Expand All @@ -26,11 +27,15 @@ public Object postProcessAfterInitialization(Object bean, String beanName) {
}

RestTemplate restTemplate = (RestTemplate) bean;
addRestTemplateInterceptorIfNotPresent(restTemplate);
OpenTelemetry openTelemetry = openTelemetryProvider.getIfUnique();
if (openTelemetry != null) {
addRestTemplateInterceptorIfNotPresent(restTemplate, openTelemetry);
}
return restTemplate;
}

private void addRestTemplateInterceptorIfNotPresent(RestTemplate restTemplate) {
private static void addRestTemplateInterceptorIfNotPresent(
RestTemplate restTemplate, OpenTelemetry openTelemetry) {
List<ClientHttpRequestInterceptor> restTemplateInterceptors = restTemplate.getInterceptors();
if (restTemplateInterceptors.stream()
.noneMatch(interceptor -> interceptor instanceof RestTemplateInterceptor)) {
Expand Down
6 changes: 4 additions & 2 deletions ...nstrumentation/spring/autoconfigure/httpclients/webclient/WebClientAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.spring.autoconfigure.httpclients.HttpClientsProperties;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
Expand All @@ -29,7 +30,8 @@
public class WebClientAutoConfiguration {

@Bean
public WebClientBeanPostProcessor otelWebClientBeanPostProcessor(OpenTelemetry openTelemetry) {
return new WebClientBeanPostProcessor(openTelemetry);
public WebClientBeanPostProcessor otelWebClientBeanPostProcessor(
ObjectProvider<OpenTelemetry> openTelemetryProvider) {
return new WebClientBeanPostProcessor(openTelemetryProvider);
}
}
21 changes: 14 additions & 7 deletions ...nstrumentation/spring/autoconfigure/httpclients/webclient/WebClientBeanPostProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import io.opentelemetry.instrumentation.spring.webflux.client.WebClientTracingFilter;
import java.util.List;
import java.util.function.Consumer;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.web.reactive.function.client.ExchangeFilterFunction;
import org.springframework.web.reactive.function.client.WebClient;
Expand All @@ -20,27 +21,33 @@
*/
final class WebClientBeanPostProcessor implements BeanPostProcessor {

private final OpenTelemetry openTelemetry;
private final ObjectProvider<OpenTelemetry> openTelemetryProvider;

WebClientBeanPostProcessor(OpenTelemetry openTelemetry) {
this.openTelemetry = openTelemetry;
WebClientBeanPostProcessor(ObjectProvider<OpenTelemetry> openTelemetryProvider) {
this.openTelemetryProvider = openTelemetryProvider;
}

@Override
public Object postProcessAfterInitialization(Object bean, String beanName) {
if (bean instanceof WebClient) {
WebClient webClient = (WebClient) bean;
return wrapBuilder(openTelemetry, webClient.mutate()).build();
return wrapBuilder(openTelemetryProvider, webClient.mutate()).build();
} else if (bean instanceof WebClient.Builder) {
WebClient.Builder webClientBuilder = (WebClient.Builder) bean;
return wrapBuilder(openTelemetry, webClientBuilder);
return wrapBuilder(openTelemetryProvider, webClientBuilder);
}
return bean;
}

private static WebClient.Builder wrapBuilder(
OpenTelemetry openTelemetry, WebClient.Builder webClientBuilder) {
return webClientBuilder.filters(webClientFilterFunctionConsumer(openTelemetry));
ObjectProvider<OpenTelemetry> openTelemetryProvider, WebClient.Builder webClientBuilder) {

OpenTelemetry openTelemetry = openTelemetryProvider.getIfUnique();
if (openTelemetry != null) {
return webClientBuilder.filters(webClientFilterFunctionConsumer(openTelemetry));
} else {
return webClientBuilder;
}
}

private static Consumer<List<ExchangeFilterFunction>> webClientFilterFunctionConsumer(
Expand Down
46 changes: 44 additions & 2 deletions ...tion/spring/autoconfigure/httpclients/resttemplate/RestTemplateBeanPostProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,33 @@
package io.opentelemetry.instrumentation.spring.autoconfigure.httpclients.resttemplate;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.spring.httpclients.RestTemplateInterceptor;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.web.client.RestTemplate;

@ExtendWith(MockitoExtension.class)
class RestTemplateBeanPostProcessorTest {

RestTemplateBeanPostProcessor restTemplateBeanPostProcessor =
new RestTemplateBeanPostProcessor(OpenTelemetry.noop());
@Mock ObjectProvider<OpenTelemetry> openTelemetryProvider;

RestTemplateBeanPostProcessor restTemplateBeanPostProcessor;

@BeforeEach
void setUp() {
restTemplateBeanPostProcessor = new RestTemplateBeanPostProcessor(openTelemetryProvider);
}

@Test
@DisplayName("when processed bean is not of type RestTemplate should return object")
Expand All @@ -28,20 +41,28 @@ void returnsObject() {
restTemplateBeanPostProcessor.postProcessAfterInitialization(
new Object(), "testObject"))
.isExactlyInstanceOf(Object.class);

verifyNoInteractions(openTelemetryProvider);
}

@Test
@DisplayName("when processed bean is of type RestTemplate should return RestTemplate")
void returnsRestTemplate() {
when(openTelemetryProvider.getIfUnique()).thenReturn(OpenTelemetry.noop());

assertThat(
restTemplateBeanPostProcessor.postProcessAfterInitialization(
new RestTemplate(), "testRestTemplate"))
.isInstanceOf(RestTemplate.class);

verify(openTelemetryProvider).getIfUnique();
}

@Test
@DisplayName("when processed bean is of type RestTemplate should add ONE RestTemplateInterceptor")
void addsRestTemplateInterceptor() {
when(openTelemetryProvider.getIfUnique()).thenReturn(OpenTelemetry.noop());

RestTemplate restTemplate = new RestTemplate();

restTemplateBeanPostProcessor.postProcessAfterInitialization(restTemplate, "testRestTemplate");
Expand All @@ -53,5 +74,26 @@ void addsRestTemplateInterceptor() {
.filter(rti -> rti instanceof RestTemplateInterceptor)
.count())
.isEqualTo(1);

verify(openTelemetryProvider, times(3)).getIfUnique();
}

@Test
@DisplayName("when OpenTelemetry is not available should NOT add RestTemplateInterceptor")
void doesNotAddRestTemplateInterceptorIfOpenTelemetryUnavailable() {
when(openTelemetryProvider.getIfUnique()).thenReturn(null);
RestTemplate restTemplate = new RestTemplate();

restTemplateBeanPostProcessor.postProcessAfterInitialization(restTemplate, "testRestTemplate");
restTemplateBeanPostProcessor.postProcessAfterInitialization(restTemplate, "testRestTemplate");
restTemplateBeanPostProcessor.postProcessAfterInitialization(restTemplate, "testRestTemplate");

assertThat(
restTemplate.getInterceptors().stream()
.filter(rti -> rti instanceof RestTemplateInterceptor)
.count())
.isEqualTo(0);

verify(openTelemetryProvider, times(3)).getIfUnique();
}
}
59 changes: 57 additions & 2 deletions ...umentation/spring/autoconfigure/httpclients/webclient/WebClientBeanPostProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,33 @@
package io.opentelemetry.instrumentation.spring.autoconfigure.httpclients.webclient;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.spring.webflux.client.WebClientTracingFilter;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.web.reactive.function.client.WebClient;

@ExtendWith(MockitoExtension.class)
class WebClientBeanPostProcessorTest {

WebClientBeanPostProcessor webClientBeanPostProcessor =
new WebClientBeanPostProcessor(OpenTelemetry.noop());
@Mock ObjectProvider<OpenTelemetry> openTelemetryProvider;

WebClientBeanPostProcessor webClientBeanPostProcessor;

@BeforeEach
void setUp() {
webClientBeanPostProcessor = new WebClientBeanPostProcessor(openTelemetryProvider);
}

@Test
@DisplayName(
Expand All @@ -29,29 +42,41 @@ void returnsObject() {
assertThat(
webClientBeanPostProcessor.postProcessAfterInitialization(new Object(), "testObject"))
.isExactlyInstanceOf(Object.class);

verifyNoInteractions(openTelemetryProvider);
}

@Test
@DisplayName("when processed bean is of type WebClient should return WebClient")
void returnsWebClient() {
when(openTelemetryProvider.getIfUnique()).thenReturn(OpenTelemetry.noop());

assertThat(
webClientBeanPostProcessor.postProcessAfterInitialization(
WebClient.create(), "testWebClient"))
.isInstanceOf(WebClient.class);

verify(openTelemetryProvider).getIfUnique();
}

@Test
@DisplayName("when processed bean is of type WebClientBuilder should return WebClientBuilder")
void returnsWebClientBuilder() {
when(openTelemetryProvider.getIfUnique()).thenReturn(OpenTelemetry.noop());

assertThat(
webClientBeanPostProcessor.postProcessAfterInitialization(
WebClient.builder(), "testWebClientBuilder"))
.isInstanceOf(WebClient.Builder.class);

verify(openTelemetryProvider).getIfUnique();
}

@Test
@DisplayName("when processed bean is of type WebClient should add exchange filter to WebClient")
void addsExchangeFilterWebClient() {
when(openTelemetryProvider.getIfUnique()).thenReturn(OpenTelemetry.noop());

WebClient webClient = WebClient.create();
Object processedWebClient =
webClientBeanPostProcessor.postProcessAfterInitialization(webClient, "testWebClient");
Expand All @@ -67,12 +92,40 @@ void addsExchangeFilterWebClient() {
.count())
.isEqualTo(1);
});

verify(openTelemetryProvider).getIfUnique();
}

@Test
@DisplayName(
"when processed bean is of type WebClient and OpenTelemetry is not available should NOT add exchange filter to WebClient")
void doesNotAddExchangeFilterWebClientIfOpenTelemetryUnavailable() {
when(openTelemetryProvider.getIfUnique()).thenReturn(null);

WebClient webClient = WebClient.create();
Object processedWebClient =
webClientBeanPostProcessor.postProcessAfterInitialization(webClient, "testWebClient");

assertThat(processedWebClient).isInstanceOf(WebClient.class);
((WebClient) processedWebClient)
.mutate()
.filters(
functions -> {
assertThat(
functions.stream()
.filter(wctf -> wctf instanceof WebClientTracingFilter)
.count())
.isEqualTo(0);
});

verify(openTelemetryProvider).getIfUnique();
}

@Test
@DisplayName(
"when processed bean is of type WebClientBuilder should add ONE exchange filter to WebClientBuilder")
void addsExchangeFilterWebClientBuilder() {
when(openTelemetryProvider.getIfUnique()).thenReturn(OpenTelemetry.noop());

WebClient.Builder webClientBuilder = WebClient.builder();
webClientBeanPostProcessor.postProcessAfterInitialization(
Expand All @@ -88,5 +141,7 @@ void addsExchangeFilterWebClientBuilder() {
functions.stream().filter(wctf -> wctf instanceof WebClientTracingFilter).count())
.isEqualTo(1);
});

verify(openTelemetryProvider, times(3)).getIfUnique();
}
}