Skip to content

Commit

Permalink
Apply user-provided ObservationConventions in auto-configurations
Browse files Browse the repository at this point in the history
Prior to this commit, we would advise developers, as migration path from
Spring Boot 2.0-x metrics, to create `GlobalObservationConvention` beans
for the observations they want to customize (observation name or key
values).

`GlobalObservationConvention` are currently applied **in addition** to
the chosen convention in some cases, so this does not work well with
this migration path.

Instead, instrumentations always provide a default convention but also a
way to configure a custom convention for their observations. Spring Boot
should inject custom convention beans in the relevant
auto-configurations.

Fixes gh-33285
  • Loading branch information
bclozel committed Nov 22, 2022
1 parent f6ac891 commit 07766c4
Show file tree
Hide file tree
Showing 11 changed files with 238 additions and 28 deletions.
Expand Up @@ -20,6 +20,7 @@
import io.micrometer.observation.Observation;
import io.micrometer.observation.ObservationRegistry;

import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfiguration;
import org.springframework.boot.autoconfigure.AutoConfiguration;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
Expand All @@ -28,6 +29,8 @@
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.context.annotation.Bean;
import org.springframework.graphql.execution.GraphQlSource;
import org.springframework.graphql.observation.DataFetcherObservationConvention;
import org.springframework.graphql.observation.ExecutionRequestObservationConvention;
import org.springframework.graphql.observation.GraphQlObservationInstrumentation;

/**
Expand All @@ -45,9 +48,11 @@ public class GraphQlObservationAutoConfiguration {

@Bean
@ConditionalOnMissingBean
public GraphQlObservationInstrumentation graphQlObservationInstrumentation(
ObservationRegistry observationRegistry) {
return new GraphQlObservationInstrumentation(observationRegistry);
public GraphQlObservationInstrumentation graphQlObservationInstrumentation(ObservationRegistry observationRegistry,
ObjectProvider<ExecutionRequestObservationConvention> executionConvention,
ObjectProvider<DataFetcherObservationConvention> dataFetcherConvention) {
return new GraphQlObservationInstrumentation(observationRegistry, executionConvention.getIfAvailable(),
dataFetcherConvention.getIfAvailable());
}

}
Expand Up @@ -45,16 +45,34 @@ class RestTemplateObservationConfiguration {

@Bean
ObservationRestTemplateCustomizer observationRestTemplateCustomizer(ObservationRegistry observationRegistry,
ObjectProvider<ClientRequestObservationConvention> customConvention,
ObservationProperties observationProperties, MetricsProperties metricsProperties,
ObjectProvider<RestTemplateExchangeTagsProvider> optionalTagsProvider) {
String name = observationName(observationProperties, metricsProperties);
ClientRequestObservationConvention observationConvention = createConvention(customConvention.getIfAvailable(),
name, optionalTagsProvider.getIfAvailable());
return new ObservationRestTemplateCustomizer(observationRegistry, observationConvention);
}

private static String observationName(ObservationProperties observationProperties,
MetricsProperties metricsProperties) {
String metricName = metricsProperties.getWeb().getClient().getRequest().getMetricName();
String observationName = observationProperties.getHttp().getClient().getRequests().getName();
String name = (observationName != null) ? observationName : metricName;
RestTemplateExchangeTagsProvider tagsProvider = optionalTagsProvider.getIfAvailable();
ClientRequestObservationConvention observationConvention = (tagsProvider != null)
? new ClientHttpObservationConventionAdapter(name, tagsProvider)
: new DefaultClientRequestObservationConvention(name);
return new ObservationRestTemplateCustomizer(observationRegistry, observationConvention);
return (observationName != null) ? observationName : metricName;
}

private static ClientRequestObservationConvention createConvention(
ClientRequestObservationConvention customConvention, String name,
RestTemplateExchangeTagsProvider tagsProvider) {
if (customConvention != null) {
return customConvention;
}
else if (tagsProvider != null) {
return new ClientHttpObservationConventionAdapter(name, tagsProvider);
}
else {
return new DefaultClientRequestObservationConvention(name);
}
}

}
Expand Up @@ -42,16 +42,34 @@ class WebClientObservationConfiguration {

@Bean
ObservationWebClientCustomizer observationWebClientCustomizer(ObservationRegistry observationRegistry,
ObservationProperties observationProperties,
ObjectProvider<WebClientExchangeTagsProvider> optionalTagsProvider, MetricsProperties metricsProperties) {
ObjectProvider<ClientRequestObservationConvention> customConvention,
ObservationProperties observationProperties, ObjectProvider<WebClientExchangeTagsProvider> tagsProvider,
MetricsProperties metricsProperties) {
String name = observationName(observationProperties, metricsProperties);
ClientRequestObservationConvention observationConvention = createConvention(customConvention.getIfAvailable(),
tagsProvider.getIfAvailable(), name);
return new ObservationWebClientCustomizer(observationRegistry, observationConvention);
}

private static ClientRequestObservationConvention createConvention(
ClientRequestObservationConvention customConvention, WebClientExchangeTagsProvider tagsProvider,
String name) {
if (customConvention != null) {
return customConvention;
}
else if (tagsProvider != null) {
return new ClientObservationConventionAdapter(name, tagsProvider);
}
else {
return new DefaultClientRequestObservationConvention(name);
}
}

private static String observationName(ObservationProperties observationProperties,
MetricsProperties metricsProperties) {
String metricName = metricsProperties.getWeb().getClient().getRequest().getMetricName();
String observationName = observationProperties.getHttp().getClient().getRequests().getName();
String name = (observationName != null) ? observationName : metricName;
WebClientExchangeTagsProvider tagsProvider = optionalTagsProvider.getIfAvailable();
ClientRequestObservationConvention observationConvention = (tagsProvider != null)
? new ClientObservationConventionAdapter(name, tagsProvider)
: new DefaultClientRequestObservationConvention(name);
return new ObservationWebClientCustomizer(observationRegistry, observationConvention);
return (observationName != null) ? observationName : metricName;
}

}
Expand Up @@ -78,19 +78,25 @@ public WebFluxObservationAutoConfiguration(MetricsProperties metricsProperties,
@Bean
@ConditionalOnMissingBean
public ServerHttpObservationFilter webfluxObservationFilter(ObservationRegistry registry,
ObjectProvider<ServerRequestObservationConvention> customConvention,
ObjectProvider<WebFluxTagsProvider> tagConfigurer,
ObjectProvider<WebFluxTagsContributor> contributorsProvider) {
String observationName = this.observationProperties.getHttp().getServer().getRequests().getName();
String metricName = this.metricsProperties.getWeb().getServer().getRequest().getMetricName();
String name = (observationName != null) ? observationName : metricName;
WebFluxTagsProvider tagsProvider = tagConfigurer.getIfAvailable();
List<WebFluxTagsContributor> tagsContributors = contributorsProvider.orderedStream().toList();
ServerRequestObservationConvention convention = createConvention(name, tagsProvider, tagsContributors);
ServerRequestObservationConvention convention = createConvention(customConvention.getIfAvailable(), name,
tagsProvider, tagsContributors);
return new ServerHttpObservationFilter(registry, convention);
}

private ServerRequestObservationConvention createConvention(String name, WebFluxTagsProvider tagsProvider,
private static ServerRequestObservationConvention createConvention(
ServerRequestObservationConvention customConvention, String name, WebFluxTagsProvider tagsProvider,
List<WebFluxTagsContributor> tagsContributors) {
if (customConvention != null) {
return customConvention;
}
if (tagsProvider != null) {
return new ServerRequestObservationConventionAdapter(name, tagsProvider);
}
Expand Down
Expand Up @@ -82,22 +82,33 @@ public WebMvcObservationAutoConfiguration(ObservationProperties observationPrope
@Bean
@ConditionalOnMissingFilterBean
public FilterRegistrationBean<ServerHttpObservationFilter> webMvcObservationFilter(ObservationRegistry registry,
ObjectProvider<ServerRequestObservationConvention> customConvention,
ObjectProvider<WebMvcTagsProvider> customTagsProvider,
ObjectProvider<WebMvcTagsContributor> contributorsProvider) {
String name = httpRequestsMetricName(this.observationProperties, this.metricsProperties);
ServerRequestObservationConvention convention = new DefaultServerRequestObservationConvention(name);
WebMvcTagsProvider tagsProvider = customTagsProvider.getIfAvailable();
List<WebMvcTagsContributor> contributors = contributorsProvider.orderedStream().toList();
if (tagsProvider != null || contributors.size() > 0) {
convention = new ServerRequestObservationConventionAdapter(name, tagsProvider, contributors);
}
ServerRequestObservationConvention convention = createConvention(customConvention.getIfAvailable(), name,
customTagsProvider.getIfAvailable(), contributorsProvider.orderedStream().toList());
ServerHttpObservationFilter filter = new ServerHttpObservationFilter(registry, convention);
FilterRegistrationBean<ServerHttpObservationFilter> registration = new FilterRegistrationBean<>(filter);
registration.setOrder(Ordered.HIGHEST_PRECEDENCE + 1);
registration.setDispatcherTypes(DispatcherType.REQUEST, DispatcherType.ASYNC);
return registration;
}

private static ServerRequestObservationConvention createConvention(
ServerRequestObservationConvention customConvention, String name, WebMvcTagsProvider tagsProvider,
List<WebMvcTagsContributor> contributors) {
if (customConvention != null) {
return customConvention;
}
else if (tagsProvider != null || contributors.size() > 0) {
return new ServerRequestObservationConventionAdapter(name, tagsProvider, contributors);
}
else {
return new DefaultServerRequestObservationConvention(name);
}
}

private static String httpRequestsMetricName(ObservationProperties observationProperties,
MetricsProperties metricsProperties) {
String observationName = observationProperties.getHttp().getServer().getRequests().getName();
Expand Down
Expand Up @@ -24,6 +24,8 @@
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.graphql.observation.DefaultDataFetcherObservationConvention;
import org.springframework.graphql.observation.DefaultExecutionRequestObservationConvention;
import org.springframework.graphql.observation.GraphQlObservationInstrumentation;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -58,6 +60,18 @@ void instrumentationBacksOffIfAlreadyPresent() {
.hasBean("customInstrumentation"));
}

@Test
void instrumentationUsesCustomConventionsIfAvailable() {
this.contextRunner.withUserConfiguration(CustomConventionsConfiguration.class).run((context) -> {
GraphQlObservationInstrumentation instrumentation = context
.getBean(GraphQlObservationInstrumentation.class);
assertThat(instrumentation).extracting("requestObservationConvention")
.isInstanceOf(CustomExecutionRequestObservationConvention.class);
assertThat(instrumentation).extracting("dataFetcherObservationConvention")
.isInstanceOf(CustomDataFetcherObservationConvention.class);
});
}

@Configuration(proxyBeanMethods = false)
static class InstrumentationConfiguration {

Expand All @@ -68,4 +82,27 @@ GraphQlObservationInstrumentation customInstrumentation(ObservationRegistry regi

}

@Configuration(proxyBeanMethods = false)
static class CustomConventionsConfiguration {

@Bean
CustomExecutionRequestObservationConvention customExecutionConvention() {
return new CustomExecutionRequestObservationConvention();
}

@Bean
CustomDataFetcherObservationConvention customDataFetcherConvention() {
return new CustomDataFetcherObservationConvention();
}

}

static class CustomExecutionRequestObservationConvention extends DefaultExecutionRequestObservationConvention {

}

static class CustomDataFetcherObservationConvention extends DefaultDataFetcherObservationConvention {

}

}
Expand Up @@ -16,6 +16,7 @@

package org.springframework.boot.actuate.autoconfigure.observation.web.client;

import io.micrometer.common.KeyValues;
import io.micrometer.core.instrument.Tag;
import io.micrometer.core.instrument.Tags;
import io.micrometer.observation.ObservationRegistry;
Expand All @@ -41,6 +42,8 @@
import org.springframework.http.HttpRequest;
import org.springframework.http.HttpStatus;
import org.springframework.http.client.ClientHttpResponse;
import org.springframework.http.client.observation.ClientRequestObservationContext;
import org.springframework.http.client.observation.DefaultClientRequestObservationConvention;
import org.springframework.test.web.client.MockRestServiceServer;
import org.springframework.web.client.RestTemplate;

Expand Down Expand Up @@ -116,6 +119,17 @@ void restTemplateCreatedWithBuilderUsesCustomTagsProvider() {
});
}

@Test
void restTemplateCreatedWithBuilderUsesCustomConvention() {
this.contextRunner.withUserConfiguration(CustomConvention.class).run((context) -> {
RestTemplate restTemplate = buildRestTemplate(context);
restTemplate.getForEntity("/projects/{project}", Void.class, "spring-boot");
TestObservationRegistry registry = context.getBean(TestObservationRegistry.class);
TestObservationRegistryAssert.assertThat(registry).hasObservationWithNameEqualTo("http.client.requests")
.that().hasLowCardinalityKeyValue("project", "spring-boot");
});
}

@Test
void afterMaxUrisReachedFurtherUrisAreDenied(CapturedOutput output) {
this.contextRunner.with(MetricsRun.simple()).withPropertyValues("management.metrics.web.client.max-uri-tags=2")
Expand Down Expand Up @@ -153,7 +167,7 @@ private RestTemplate buildRestTemplate(AssertableApplicationContext context) {
return restTemplate;
}

@Configuration
@Configuration(proxyBeanMethods = false)
static class CustomTagsConfiguration {

@Bean
Expand All @@ -173,4 +187,23 @@ public Iterable<Tag> getTags(String urlTemplate, HttpRequest request, ClientHttp

}

@Configuration(proxyBeanMethods = false)
static class CustomConventionConfiguration {

@Bean
CustomConvention customConvention() {
return new CustomConvention();
}

}

static class CustomConvention extends DefaultClientRequestObservationConvention {

@Override
public KeyValues getLowCardinalityKeyValues(ClientRequestObservationContext context) {
return super.getLowCardinalityKeyValues(context).and("project", "spring-boot");
}

}

}
Expand Up @@ -18,6 +18,7 @@

import java.time.Duration;

import io.micrometer.common.KeyValues;
import io.micrometer.observation.ObservationRegistry;
import io.micrometer.observation.tck.TestObservationRegistry;
import io.micrometer.observation.tck.TestObservationRegistryAssert;
Expand All @@ -41,6 +42,8 @@
import org.springframework.http.HttpStatus;
import org.springframework.http.client.reactive.ClientHttpConnector;
import org.springframework.mock.http.client.reactive.MockClientHttpResponse;
import org.springframework.web.reactive.function.client.ClientRequestObservationContext;
import org.springframework.web.reactive.function.client.DefaultClientRequestObservationConvention;
import org.springframework.web.reactive.function.client.WebClient;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -84,6 +87,20 @@ void shouldNotOverrideCustomTagsProvider() {
.getBeans(WebClientExchangeTagsProvider.class).hasSize(1).containsKey("customTagsProvider"));
}

@Test
void shouldUseCustomConventionIfAvailable() {
this.contextRunner.withUserConfiguration(CustomConvention.class).run((context) -> {
TestObservationRegistry registry = context.getBean(TestObservationRegistry.class);
WebClient.Builder builder = context.getBean(WebClient.Builder.class);
WebClient webClient = mockWebClient(builder);
TestObservationRegistryAssert.assertThat(registry).doesNotHaveAnyObservation();
webClient.get().uri("https://example.org/projects/{project}", "spring-boot").retrieve().toBodilessEntity()
.block(Duration.ofSeconds(30));
TestObservationRegistryAssert.assertThat(registry).hasObservationWithNameEqualTo("http.client.requests")
.that().hasLowCardinalityKeyValue("project", "spring-boot");
});
}

@Test
void afterMaxUrisReachedFurtherUrisAreDenied(CapturedOutput output) {
this.contextRunner.withPropertyValues("management.metrics.web.client.max-uri-tags=2").run((context) -> {
Expand Down Expand Up @@ -141,4 +158,23 @@ WebClientExchangeTagsProvider customTagsProvider() {

}

@Configuration(proxyBeanMethods = false)
static class CustomConventionConfig {

@Bean
CustomConvention customConvention() {
return new CustomConvention();
}

}

static class CustomConvention extends DefaultClientRequestObservationConvention {

@Override
public KeyValues getLowCardinalityKeyValues(ClientRequestObservationContext context) {
return super.getLowCardinalityKeyValues(context).and("project", "spring-boot");
}

}

}

0 comments on commit 07766c4

Please sign in to comment.