Skip to content

[ISSUE #4390] Realize the SPI extension loading of RabbitMQ admin-api#4395

Merged
mxsm merged 7 commits intoapache:masterfrom
Pil0tXia:pil0txia_feat_4390
Aug 26, 2023
Merged

[ISSUE #4390] Realize the SPI extension loading of RabbitMQ admin-api#4395
mxsm merged 7 commits intoapache:masterfrom
Pil0tXia:pil0txia_feat_4390

Conversation

@Pil0tXia
Copy link
Member

Fixes #4390.

Motivation

image

StartUp failure when using RabbitMQ as storage-plugin.

Modifications

Realize the RabbitMQ admin-api to fulfuill SPI extension loading.

Admin functions?

Based on the current implementation of org.apache.eventmesh.storage.rabbitmq.producer.RabbitmqProducer, it is not possible to display message counts per topic.

RabbitMQ itself does not retain consumed messages or record topic names; it can only list exchanges and queues. The existing exchanges and queues are both singular and fixed.

To display topics, it would be necessary to retrieve the topic name from each message and use it to declare a corresponding queue. However, this approach would incur additional overhead when receiving new topics.

While elevating the topic concept to a queue could address the issue of "being able to track historical message counts for queues but not for topics," this approach aligns with the design philosophy of exchanges with TOPIC type. However, considering the current limited demand within the community, I plan to address startup errors and provide only limited management functionalities for now.

Upon investigation, it appears that utilizing the RabbitMQ Management HTTP API is a favorable approach to list queues and historical message counts.

Relevant Bug

#4394

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Merging #4395 (3dcb8f2) into master (50d6113) will decrease coverage by 0.03%.
Report is 4 commits behind head on master.
The diff coverage is 0.00%.

❗ Current head 3dcb8f2 differs from pull request most recent head 62c22e0. Consider uploading reports for the commit 62c22e0 to get more accurate results

@@             Coverage Diff              @@
##             master    #4395      +/-   ##
============================================
- Coverage     17.85%   17.82%   -0.03%     
  Complexity     1514     1514              
============================================
  Files           602      604       +2     
  Lines         25518    25550      +32     
  Branches       2400     2400              
============================================
  Hits           4555     4555              
- Misses        20524    20556      +32     
  Partials        439      439              
Files Changed Coverage Δ
...otocol/catalog/protos/QueryOperationsResponse.java 0.00% <0.00%> (ø)
...mmon/protocol/catalog/protos/RegistryResponse.java 0.00% <0.00%> (ø)
...ventmesh/storage/rabbitmq/admin/RabbitMQAdmin.java 0.00% <0.00%> (ø)
...h/storage/rabbitmq/admin/RabbitMQAdminAdaptor.java 0.00% <0.00%> (ø)
...h/storage/rabbitmq/config/ConfigurationHolder.java 76.92% <0.00%> (-13.99%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

rabbitmqctl add_user full_access s3crEt
# tag the user with "administrator" for full management UI and HTTP API access
rabbitmqctl set_user_tags full_access administrator
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are all provided by RabbitMQ. I would like to know the relationship between these and eventmesh-dashboard.

Copy link
Member

@pandaapo pandaapo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that the implementation of admin is all empty, so this PR is an initialization work, right?

public List<TopicProperties> getTopic() {
// Utilizing the RabbitMQ Management HTTP API is a favorable approach to list queues and historical message counts.
// To display topics, it would be necessary to retrieve the topic name from each message and use it to declare a corresponding queue.
return new ArrayList<>();
Copy link
Member

@pandaapo pandaapo Aug 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do these two lines of comments mean? Are they expressing "TODO" or are they expressing "Please use RabbitMQ's management features"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an instruction for future development, indicating that using RabbitMQ HTTP API provided by rabbitmq_management webui is the best way to implement this admin function.

The reason that why I didn't implement this is because that the admin function is limited by storage-plugin and the original author is currently unreachable. I will come back after other storage-plugins' admin functions are done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So these comments are expressing "TODO", right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes~

Copy link
Member

@mxsm mxsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mxsm mxsm merged commit 707c98d into apache:master Aug 26, 2023
@Pil0tXia Pil0tXia deleted the pil0txia_feat_4390 branch January 4, 2024 05:06
xuhongjia pushed a commit to Deckers-Ohana/eventmesh that referenced this pull request Mar 13, 2025
…in-api (apache#4395)

* config: rabbitmq spi without webhook, RESTORE BEFORE MERGE

* config: add extensionType in META-INF

* fix: resolve extension not found error when startup

* feat: add rabbitmq management config

* feat: provide limited admin func

* Revert "config: rabbitmq spi without webhook, RESTORE BEFORE MERGE"

This reverts commit 05daccb.

* doc: add PR link to README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Realize the SPI extension loading of RabbitMQ admin-api

4 participants