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 support for messageId, correlationId, and type in RabbitMQ bindings #3661

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

abossard
Copy link
Contributor

@abossard abossard commented Feb 5, 2025

Description

This pull request introduces several enhancements to the RabbitMQ bindings and metadata utilities. The most important changes involve adding new utility functions to extract specific metadata fields and updating the RabbitMQ publishing functions to utilize these new utilities.

Enhancements to metadata utilities:

  • metadata/utils.go: Added TryGetMessageId, TryGetCorrelationId, and TryGetType functions to extract messageId, correlationId, and type from metadata properties, respectively. [1] [2]

Updates to RabbitMQ bindings:

  • bindings/rabbitmq/rabbitmq.go: Modified the Invoke method to extract and set MessageId, CorrelationId, and Type from the request metadata using the new utility functions.
  • pubsub/rabbitmq/rabbitmq.go: Updated the publishSync method to extract and set ContentType, MessageId, CorrelationId, and Type from the request metadata using the new utility functions.

Issue reference

Please reference the issue this PR will close: #3650

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

@abossard abossard requested review from a team as code owners February 5, 2025 10:58
@abossard abossard force-pushed the supporting_more_properties_for_rabbitmq_on_main branch from f650ec4 to d0c75f6 Compare February 5, 2025 18:07
Copy link
Member

@berndverst berndverst left a comment

Choose a reason for hiding this comment

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

Please run our linter. make lint and fix lint issues.

As discussed, please do not add component specific code to metadata/utils.go - this should go into common/component/rabbitmq

Also you need to add tests:

Unit tests for Rabbit MQ Binding:

Unit tests for Rabbit MQ Pubsub:

Integration test (if applicable):

Certification Tests (required for stable components in Dapr):

The Rabbit MQ Bindings certification test should be updated to add a test case that exercises these new options:

The Rabbit MQ PubSub certification test should be updated to add a test case that exercises these new options:

To run the certification tests go into those repos and simply run go test -v --count=1 . -- if you run into an error complaining about protos you need to first run export GOLANG_PROTOBUF_REGISTRATION_CONFLICT=ignore

@@ -113,6 +113,21 @@ func IsRawPayload(props map[string]string) (bool, error) {

return false, nil
}
func TryGetMessageId(props map[string]string) (string, bool) {
Copy link
Member

Choose a reason for hiding this comment

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

This is not the right place for your RabbitMQ component specific shared code.

Instead of metadata/utils.go, create a rabbitmq folder here: https://github.com/dapr/components-contrib/tree/main/common/component

Then import from there. Please add a unit test for these helper methods.

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.

RabbitMQ Messages should have some cloud event property fields populated, e.g. message id and correlation id
2 participants