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.

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

Add RabbitMQ auto-configuration support when camel-rabbit is used instead of rabbit-amqp #17534

Open
rfelgent opened this issue Jul 16, 2019 · 5 comments
Labels
type: enhancement A general enhancement

Comments

@rfelgent
Copy link

rfelgent commented Jul 16, 2019

Hello people,

I am using "camel-rabbit" instead of "rabbit-amqp" for different reasons.
By using "camel-rabbit" I loose all the autoconfiguration of

  • MetricRegistry
  • and HealthIndicator

To make this work, I craeted my own implementations for the features in question:

/**
 * By using camel-rabbit instead of spring-amqp you loose the auto configuration feature of AMQP Rabbit health info.
 * Therefore, we do the health check ourselves.
 *
 * @see: {@link org.springframework.boot.actuate.amqp.RabbitHealthIndicator}
 */
@Component
public class RabbitHealthIndicator extends AbstractHealthIndicator {

  private ConnectionFactory connectionFactory;

  public RabbitHealthIndicator(ConnectionFactory connectionFactory) {
    this.connectionFactory = connectionFactory;
  }

  @Override
  protected void doHealthCheck(Health.Builder builder) throws Exception {
    builder.up().withDetail("version", getVersion());
  }

  private String getVersion() throws IOException, TimeoutException {
    try( Connection connection = connectionFactory.newConnection() ) {
      Map<String, Object> properties = connection.getServerProperties();
      return properties.get("version").toString();
    }
  }
}
/**
 * By using camel-rabbit instead of spring-amqp you loose the auto configuration feature of AMQP Rabbit metrics.
 * Therefore, we do the wiring for {@link ConnectionFactory} and {@link RabbitMetrics} manually
 *
 * @see: {@link org.springframework.boot.actuate.autoconfigure.metrics.amqp.RabbitConnectionFactoryMetricsPostProcessor}
 * @see  {@link org.springframework.boot.actuate.autoconfigure.metrics.amqp.RabbitMetricsAutoConfiguration}
 */
@Configuration
// TODO: why does this break the metrics configuration ?
//@ConditionalOnBean({ ConnectionFactory.class, MeterRegistry.class })
public class RabbitMetricsAutoConfiguration {

  @Autowired
  private ConnectionFactory connectionFactory;
  @Autowired
  private MeterRegistry registry;

  @PostConstruct
  public void postConstruct() {
    new RabbitMetrics(connectionFactory, Tags.of("name", RabbitMqConfig.CONNECTION_FACTORY_NAME)).bindTo(registry);
  }
}

In my experience, either using "camel-rabbit" or "spring-amqp" both rely on the "ConnectionFactory" from the "rabbit-client".
In my project, the camel-rabbit components are configured to reuse the "ConnectionFactory" available in the Spring Context.

It would be great to have the default implementation of "RabbitMetricsAutoConfiguration" and "RabbitHealthIndicator" only dependend on "ConnectionFactory".

Best regards

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

snicoll commented Jul 16, 2019

@rfelgent I've edited your comment to improve the formatting. You might want to check out this Mastering Markdown guide for future reference.

@snicoll snicoll changed the title Autoconfig AMQP: pls be open to 3rd party libaries (e.g. camel-rabbit) Add RabbitMQ auto-configuration support when camel-rabbit is used instead of rabbit-amqp Jul 16, 2019
@wilkinsona
Copy link
Member

As things stand, we don't create a com.rabbitmq.client.ConnectionFactory bean so switching RabbitMetricsAutoConfiguration and RabbitHealthIndicator to use one isn't a completely straightforward change to make. That said, it does seem like a reasonable thing to try to do. Our DataSource health indicator doesn't require a JdbcTemplate bean so it seems a little inconsistent for our Rabbit health indicator to rely on RabbitTemplate.

@rfelgent
Copy link
Author

rfelgent commented Jul 16, 2019

Hello people,

thx for you replies.

@snicoll , thx for your edits and sorry, that my formatting was not properly!
@wilkinsona . I did not think about JdbcTemplate, but your hint about the inconsistency is correct. In my opinion, this is another (good) argument for my wish to not depend on spring-amqp ...

What exactly do you mean by saying:

we don't create a com.rabbitmq.client.ConnectionFactory

Imho, you do create (or reference) the ConnectionFactory within the post processor RabbitConnectionFactoryMetricsPostProcessor. Unfortunately, this factory is from spring-amqp.

Maybe it is an idea to move the functionality of "AbstractConnectionFactory" class to spring-boot-autoconfigure and the implementation supports only the "raw" rabbit connection factory.

@wilkinsona
Copy link
Member

wilkinsona commented Jul 17, 2019

What exactly do you mean by saying:

we don't create a com.rabbitmq.client.ConnectionFactory

You missed the word "bean" off the end that is important. While we do create a ConnectionFactory it is not created as a bean and, therefore, is not available for dependency injection. This means that we can't inject it into RabbitHealthIndicator and RabbitMetricsAutoConfiguration without defining an extra bean and making some public API changes.

Maybe it is an idea to move the functionality of "AbstractConnectionFactory" class to spring-boot-autoconfigure

AbstractConnectionFactory is part of the Spring AMQP project. We can't move it into Spring Boot as it would create a circular dependency between the two projects.

@rfelgent
Copy link
Author

Hi @wilkinsona ,

AbstractConnectionFactory is part of the Spring AMQP project. We can't move it into Spring Boot as it would create a circular dependency between the two projects.

Sorry, I did not express myself very good. I know about the circular dependency. I was talking about the functionality in general (facade, abstraction over connectionfactory).

Anyway, the AbstractConnectionFactory is available as Spring managed bean, right ? If so, in my opinion it should not be a big deal to work with either bean ConnectionFactory or bean AbstractConnectionFactory.

@wilkinsona wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 12, 2020
@wilkinsona wilkinsona added this to the General Backlog milestone Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants