Skip to content

Expose Spring Data Repository metrics #22217

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.

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

Closed
mp911de opened this issue Jul 3, 2020 · 9 comments
Closed

Expose Spring Data Repository metrics #22217

mp911de opened this issue Jul 3, 2020 · 9 comments
Assignees
Labels
status: noteworthy A noteworthy issue to call out in the release notes type: enhancement A general enhancement
Milestone

Comments

@mp911de
Copy link
Member

mp911de commented Jul 3, 2020

Spring Data repository invocations are not metered yet and it would make sense to expose invocation metrics to improve observability from a repository perspective.

Ideally, repository metrics measure the number of invocations and the timing for each repository method. The metering should use a Micrometer Timer with the following tags per method invocation:

  • repository: Simple name of the repository interface class
  • method: Method name of the invoked method
  • invocation: Invocation type: SYNC (Person, List, …), ASYNC (CompletionStage, ListenableFuture), Stream (Java 8 Stream), REACTIVE (a supported Reactive type according to ReactiveAdapterRegistry)
  • result: Whether the method invocation yielded a result (NULL, EMPTY (for Optional.empty() or an empty Collection), PRESENT (for Optional.of(…)))
  • exception: Simple name of the exception if an exception was thrown

Specifics to consider:

  • Asynchronous calls: Repository queries may offload calls to a worker thread and return CompletionStage or ListenableFuture
  • Reactive repositories: Return a reactive type, metrics get collected on success or on error
  • Stream queries: Metrics get collected when the Stream is fully consumed/Stream gets closed

Metrics can be collected through a MethodInterceptor as repositories are pure proxy objects that internally dispatch method calls, so from an outside an interceptor seems appropriate. The interceptor can be attached through a RepositoryProxyPostProcessor which needs to be configured on repository factory beans (RepositoryFactoryBeanSupport). That change needs to be done in Spring Data Commons (see DATACMNS-1688).

I have a PoC that uses BeanPostProcessor.postProcessBeforeInitialization(…) so we can turn that into a pull request.

Likely, this feature would require a bit of auto-configuration since metrics so we would need to find a good spot for configuration properties.

Limitations:
JPA runs some activity that happens outside of repository calls (e.g. lazy loading, defer activities until transaction cleanup). These activities would not be included in these metrics.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 3, 2020
@wilkinsona
Copy link
Member

wilkinsona commented Jul 3, 2020

Thanks for the suggestion, @mp911de.

On first impression it sounds to me like much of the code should go in Spring Data with Spring Boot then auto-configuring things when both Micrometer and Spring Data are on the classpath. I'm imagining Spring Data providing a MicrometerMetricsMethodInterceptor or the like that we can configure when appropriate.

This would be similar to the approach that's been taken in other projects such as Spring Kafka that provides MicrometerConsumerListener and MicrometerProducerListener that we then auto-configure:

@Bean
public DefaultKafkaProducerFactoryCustomizer kafkaProducerMetrics(MeterRegistry meterRegistry) {
return (producerFactory) -> addListener(producerFactory, meterRegistry);
}
@Bean
public DefaultKafkaConsumerFactoryCustomizer kafkaConsumerMetrics(MeterRegistry meterRegistry) {
return (consumerFactory) -> addListener(consumerFactory, meterRegistry);
}
private <K, V> void addListener(DefaultKafkaConsumerFactory<K, V> factory, MeterRegistry meterRegistry) {
factory.addListener(new MicrometerConsumerListener<>(meterRegistry));
}
private <K, V> void addListener(DefaultKafkaProducerFactory<K, V> factory, MeterRegistry meterRegistry) {
factory.addListener(new MicrometerProducerListener<>(meterRegistry));
}

@mp911de
Copy link
Member Author

mp911de commented Jul 3, 2020

I’m quite reluctant to introduce Micrometer to Spring Data as we would introduce another dependency constraint on our release process. Metrics and statistics aren’t things that Spring Data had as domain and I’d like to keep it that way. We’d rather go for a similar arrangement as with WebMVC/WebFlux metrics.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Jul 6, 2020
@philwebb
Copy link
Member

philwebb commented Jul 7, 2020

Whilst I think this would be a nice addition, I don't think that we should try to add too much custom code in Spring Boot to do this. We've already been trying to move some of our MVC metrics code into Spring Framework where it has a more natural home. The specific complexities around asynchronous calls make me quite wary that we'll be maintaining a lot of code without the real expertise required to support it. It's still worth looking at the prototype code that's already been developed, just to see if my concerns are ill-founded or not.

My initial feeling is that we should look at something similar to Hikari. They have a MetricsTrackerFactory interface that creates a IMetricsTracker instance. This provides some nice high-level metrics hook points but doesn't directly tie anything to Micrometer. In that example, there's a MicrometerMetricsTracker IMetricsTracker implementation that eventually bridges to Micrometer. Spring Boot simply configures the datasource to use it.

I like that model a lot because it makes keeps metrics in the domain of the code that understands it best, but doesn't create any hard dependency on any one library.

@mp911de can you share what you have so far (it doesn't need to be a pull-request). Does your prototype code deal with the specific concerns you mentioned in the top issue comment?

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Jul 7, 2020
@mp911de
Copy link
Member Author

mp911de commented Jul 8, 2020

Thanks, Phil. Your words frame exactly the issue. I'm also concerned about the amount of code if all of this functionality would be handled by Spring Boot. We should come up with an approach that doesn't require any outside library to make the same assumptions over execution details as Spring Data does. At the same time, we need something that is able to expose all required details so Spring Boot (or any other library) can collect metrics.

Let me investigate on that topic so I can come back with an API proposal. Here's my draft that uses a MethodInterceptor. The draft deals with all concerns except for Kotlin Coroutine methods.

@wilkinsona wilkinsona added status: blocked An issue that's blocked on an external project change type: enhancement A general enhancement labels Nov 11, 2020
@wilkinsona
Copy link
Member

@mp911de How's this going on the Spring Data side?

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue status: blocked An issue that's blocked on an external project change and removed status: blocked An issue that's blocked on an external project change status: waiting-for-triage An issue we've not yet triaged labels Jan 15, 2021
@wilkinsona wilkinsona added this to the 2.x milestone Jan 15, 2021
@mp911de
Copy link
Member Author

mp911de commented Jan 15, 2021

We've provided with Spring Data 2.4 an API to listen for repository invocation along with metering the invocation duration. Here's an example: https://github.com/spring-projects/spring-data-examples/tree/master/mongodb/repository-metrics

@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 Jan 15, 2021
@wilkinsona
Copy link
Member

Cool, thanks. Let's see if we can do something with this in Spring Boot 2.5.

@wilkinsona wilkinsona removed status: blocked An issue that's blocked on an external project change status: feedback-provided Feedback has been provided labels Jan 18, 2021
@wilkinsona wilkinsona modified the milestones: 2.x, 2.5.x Jan 18, 2021
@sanjayrawat1
Copy link

sanjayrawat1 commented Apr 2, 2021

@mp911de I took the reference of your draft of using MethodInterceptor for repository metrics, which is exposing metrics but I lose the histogram config, common tags and other configurations which I configured in application.yml. Could you please guide me why this is happening. I tried annotating RepositoryMetricsConfig class with @AutoConfigureAfter(MetricsAutoConfiguration.class) but still no luck.

@sanjayrawat1
Copy link

sanjayrawat1 commented Apr 2, 2021

Below is the sample (inspired by @mp911de draft) which is working in 2.4.x version without losing histogram and other configs defined in application.yml and I believe this will also work in 2.3.x:

public class JpaMetricsRepositoryFactoryBean<T extends Repository<S, ID>, S, ID> extends JpaRepositoryFactoryBean<T, S, ID> {

	private MeterRegistry registry;

	private RepositoryTagsProvider tagsProvider;

	public JpaMetricsRepositoryFactoryBean(Class<? extends T> repositoryInterface) {
		super(repositoryInterface);
	}

	@Autowired
	public void setRegistry(ObjectProvider<MeterRegistry> registry) {
		this.registry = registry.getIfAvailable(() -> Metrics.globalRegistry);
	}

	@Autowired
	public void setTagsProvider(ObjectProvider<RepositoryTagsProvider> tagsProvider) {
		this.tagsProvider = tagsProvider.getIfAvailable(DefaultRepositoryTagsProvider::new);
	}

	@Override
	protected RepositoryFactorySupport createRepositoryFactory(EntityManager entityManager) {
		RepositoryFactorySupport repositoryFactory = super.createRepositoryFactory(entityManager);
		repositoryFactory.addRepositoryProxyPostProcessor(new RepositoryMetricsConfiguration.RepositoryMetricsProxyPostProcessor(registry, tagsProvider));
		return repositoryFactory;
	}
}
@Slf4j
@Configuration
@EnableJpaRepositories(repositoryFactoryBeanClass = JpaMetricsRepositoryFactoryBean.class)
@EnableTransactionManagement
public class DatabaseConfiguration {}
public class RepositoryMetricsConfiguration {

	@Bean
	@ConditionalOnMissingBean(RepositoryTagsProvider.class)
	public DefaultRepositoryTagsProvider repositoryTagsProvider(ObjectProvider<RepositoryTagsContributor> contributors) {
		return new DefaultRepositoryTagsProvider(contributors.orderedStream().collect(Collectors.toList()));
	}

	static class RepositoryMetricsProxyPostProcessor implements RepositoryProxyPostProcessor {

		private final MeterRegistry registry;

		private final RepositoryTagsProvider tagsProvider;

		public RepositoryMetricsProxyPostProcessor(MeterRegistry registry, RepositoryTagsProvider tagsProvider) {
			this.registry = registry;
			this.tagsProvider = tagsProvider;
		}

		@Override
		public void postProcess(ProxyFactory factory, RepositoryInformation repositoryInformation) {
			MetricsMethodInterceptor interceptor = new MetricsMethodInterceptor(
				registry,
				repositoryInformation.getRepositoryInterface(),
				tagsProvider,
				"spring.data.repositories",
				AutoTimer.ENABLED
			);
			factory.addAdvice(interceptor);
		}
	}

	static class MetricsMethodInterceptor implements MethodInterceptor {

		private final MeterRegistry registry;

		private final Class<?> repositoryInterface;

		private final RepositoryTagsProvider tagsProvider;

		private final String metricName;

		private final AutoTimer autoTimer;

		// prettier-ignore
		public MetricsMethodInterceptor(MeterRegistry registry, Class<?> repositoryInterface, RepositoryTagsProvider tagsProvider,
										String metricName, AutoTimer autoTimer) {
			this.registry = registry;
			this.repositoryInterface = repositoryInterface;
			this.tagsProvider = tagsProvider;
			this.metricName = metricName;
			this.autoTimer = autoTimer;
		}

		@Override
		public Object invoke(MethodInvocation invocation) throws Throwable {
			Timer.Sample timerSample = Timer.start(this.registry);
			try {
				Object result = invocation.proceed();
				// record
				return result;
			} catch (Throwable throwable) {
				// record
				throw throwable;
			}
		}
	}
}

@philwebb philwebb self-assigned this Apr 9, 2021
@philwebb philwebb modified the milestones: 2.5.x, 2.5.0-RC1 Apr 10, 2021
@philwebb philwebb added the status: noteworthy A noteworthy issue to call out in the release notes label Apr 10, 2021
philwebb added a commit that referenced this issue Apr 10, 2021
Rework existing `@Timer` annotation support to remove duplicate code
and offer general purpose utilities that can be used in future metrics
support.

See gh-23112
See gh-22217
philwebb added a commit that referenced this issue Apr 10, 2021
Rework the 'Supported Metrics' documentation to improve consistency
access subsections. Details about the `@Timer` annotation have been
pulled into a new section so that they can be referenced rather than
repeated.

See gh-22217
izeye added a commit to izeye/spring-boot that referenced this issue Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: noteworthy A noteworthy issue to call out in the release notes type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants