Skip to content

Conversation

acogoluegnes
Copy link
Contributor

Enable RabbitMQ Java Client metrics collection if RabbitMQ Java Client 4.0 and Dropwizard Metrics are on the classpath. Configuration flag can also enable/disable (default is to enable).

spring-projects/spring-amqp#507 needs to be merged first.

Fixes #7498

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 28, 2016
@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 28, 2016
@philwebb
Copy link
Member

We'll need to consider the Rabbit client 4.0 upgrade (#7509) before we can do this.

@philwebb
Copy link
Member

I wonder if we could write our own MetricsCollector implementation so that metrics could be added even if coda metrics wasn't being used. That might be a little tricky since we'd need to do it in spring-boot-actuator and not in spring-boot-autoconfiguration. Perhaps we need a customizer callback for com.rabbitmq.client.ConnectionFactory.

@philwebb philwebb added status: on-hold We can't start working on this issue yet for: team-attention An issue we'd like other members of the team to review labels Nov 28, 2016
@philwebb
Copy link
Member

@acogoluegnes I'm not sure if you have time, but perhaps you could investigate a Spring Boot specific MetricsCollector?

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Nov 30, 2016
@acogoluegnes
Copy link
Contributor Author

@philwebb Do you mean something based on Spring Boot's CounterService? That's certainly doable, but we'd need to refactor the implementation that ships with RabbitMQ client to be able to re-use the tricky parts through an abstract class and let sub-classes just increment their metrics. This could be done in 4.0.1. Does that make sense?

@philwebb
Copy link
Member

philwebb commented Dec 1, 2016

Does that make sense?

Yeah, that would be ideal. I was thinking we'd need to copy/paste a bunch of that code but an abstract base class would be much better.

@acogoluegnes
Copy link
Contributor Author

I created the issue in the Java Client. How would you like to proceed? Should it be included in the current PR or somewhere else?

@philwebb
Copy link
Member

philwebb commented Dec 5, 2016

The current PR would be best thanks. You can force push and update to the branch.

@acogoluegnes
Copy link
Contributor Author

@philwebb Here is the CounterService-based implementation of MetricsCollector. Are you OK with the implementation? If yes, I need to release RabbitMQ Java Client 4.1.0 (the PR uses a snapshot) and ask the Spring AMQP folks to use it in Spring AMQP 2.0.

@philwebb philwebb added this to the 2.0.0 milestone Jan 27, 2017
@acogoluegnes
Copy link
Contributor Author

@philwebb Any update on this? Please let me know if you're fine with the implementation (especially the ConnectionFactoryCustomizer trick). I'd like to release RabbitMQ Java Client 4.1.0 (which will contain the metrics collector abstract class). No need to merge now, green light is enough. Thanks.

@acogoluegnes
Copy link
Contributor Author

PR is now based on RabbitMQ Java Client 4.1.0 (through Spring AMQP 2.0), not just a snapshot.

@philwebb
Copy link
Member

philwebb commented Nov 8, 2017

Duplicates #10887

@philwebb philwebb closed this Nov 8, 2017
@philwebb philwebb added the status: duplicate A duplicate of another issue label Nov 8, 2017
@philwebb philwebb removed status: on-hold We can't start working on this issue yet type: enhancement A general enhancement labels Nov 8, 2017
@philwebb philwebb removed this from the 2.0.0 milestone Nov 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants