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

Sync AMQP 1.0 Application Properties and AMQP 0.9.1 Properties/Headers in RabbitMQ Shovel #7508

Closed
Haigutus opened this issue Mar 2, 2023 · 5 comments

Comments

@Haigutus
Copy link

Haigutus commented Mar 2, 2023

https://groups.google.com/g/rabbitmq-users/c/2RhTKTYKc7I/m/_dAH1cUqBQAJ

RabbitMQ Shovel currently does not keep the AMQP 1.0 Application Properties and AMQP 0.9.1 Properties/Headers in sync when copying messages between exchanges or queues when transferring messages between AMQP 1.0 and AMQP 0.9.1 brokers.

To improve the functionality of RabbitMQ Shovel, I suggest that the Shovel should always copy the Application Properties and Properties/Headers of messages along with their body, and keep them in sync across all copies of the message. This will ensure that headers can be used for routing when transferring messages between AMQP 1.0 and AMQP 0.9.1 brokers.

@michaelklishin
Copy link
Member

Those properties do not have a perfect 1:1 match. At some point we plan to use AMQP 1.0 properties internally for all messages but we are not there yet.

So this is significantly more involved than it may seem.

@Haigutus
Copy link
Author

Haigutus commented Mar 2, 2023

Hi those are user defined key and value pairs, so no need to map anything, just carry over.

It seems that for AMQP 0.9.1 -> AMQP 1.0 it is actually already done, but other way it seems not, but I am not familiar enough with the code base

amqp10_msg:set_application_properties(Headers, M);

@Haigutus
Copy link
Author

Haigutus commented Mar 2, 2023

For reference

AMQP 1.0

Application Properties:

Any application-specific data, encoded as key-value pairs.

AMQP 0.9.1

Properties/Headers:
Additional metadata about the message, encoded as key-value pairs.

@Haigutus
Copy link
Author

Haigutus commented May 4, 2023

Also it seems, that for AMQP 1.0 plugin this has already been implemented.
https://github.com/rabbitmq/rabbitmq-server/blob/main/deps/rabbitmq_amqp1_0/README.md
image

Do these settings also apply for shovel?

luos added a commit to esl/rabbitmq-server that referenced this issue Dec 1, 2023
- Timestamps are milliseconds in AMQP 1.0, but in AMQP 0.9.1 it is seconds.
  Fixed by multiplying the timestamp by 1 000.
- Shovel crashed if user_id was set in the message because the encoding
  was as utf8 while it should be a byte array.
- Negative integers were encoded as integers - therefore leading to
  incorrect positive values.
- Float values were not supported by the client.
- Fixed priority header encoding in AMQP 1.0. It was set as uint but it
  should be ubyte.
- Priority of the message is now in the Headers instead of Application
  Properties. This is potentially a breaking change.

Fixes: rabbitmq#7508
olikasg added a commit to esl/rabbitmq-server that referenced this issue Dec 4, 2023
- Timestamps are milliseconds in AMQP 1.0, but in AMQP 0.9.1 it is seconds.
  Fixed by multiplying the timestamp by 1 000.
- Shovel crashed if user_id was set in the message because the encoding
  was as utf8 while it should be a byte array.
- Negative integers were encoded as integers - therefore leading to
  incorrect positive values.
- Float values were not supported by the client.
- Fixed priority header encoding in AMQP 1.0. It was set as uint but it
  should be ubyte.
- Priority of the message is now in the Headers instead of Application
  Properties. This is potentially a breaking change.

Fixes: rabbitmq#7508
luos pushed a commit to esl/rabbitmq-server that referenced this issue Dec 4, 2023
- Timestamps are milliseconds in AMQP 1.0, but in AMQP 0.9.1 it is seconds.
  Fixed by multiplying the timestamp by 1 000.
- Shovel crashed if user_id was set in the message because the encoding
  was as utf8 while it should be a byte array.
- Negative integers were encoded as integers - therefore leading to
  incorrect positive values.
- Float values were not supported by the client.
- Fixed priority header encoding in AMQP 1.0. It was set as uint but it
  should be ubyte.
- Priority of the message is now in the Headers instead of Application
  Properties. This is potentially a breaking change.

Fixes: rabbitmq#7508
luos pushed a commit to esl/rabbitmq-server that referenced this issue Dec 4, 2023
- Timestamps are milliseconds in AMQP 1.0, but in AMQP 0.9.1 it is seconds.
  Fixed by multiplying the timestamp by 1 000.
- Shovel crashed if user_id was set in the message because the encoding
  was as utf8 while it should be a byte array.
- Negative integers were encoded as integers - therefore leading to
  incorrect positive values.
- Float values were not supported by the client.
- Fixed priority header encoding in AMQP 1.0. It was set as uint but it
  should be ubyte.
- Priority of the message is now in the Headers instead of Application
  Properties. This is potentially a breaking change.

Fixes: rabbitmq#7508
luos pushed a commit to esl/rabbitmq-server that referenced this issue Dec 4, 2023
- Timestamps are milliseconds in AMQP 1.0, but in AMQP 0.9.1 it is seconds.
  Fixed by multiplying the timestamp by 1 000.
- Shovel crashed if user_id was set in the message because the encoding
  was as utf8 while it should be a byte array.
- Negative integers were encoded as integers - therefore leading to
  incorrect positive values.
- Float values were not supported by the client.
- Fixed priority header encoding in AMQP 1.0. It was set as uint but it
  should be ubyte.
- Priority of the message is now in the Headers instead of Application
  Properties. This is potentially a breaking change.

Fixes: rabbitmq#7508
@michaelklishin michaelklishin added this to the 3.12.11 milestone Dec 5, 2023
@michaelklishin
Copy link
Member

This was addressed (without any configuration settings) in #10037 for 3.12.x.

As of right now, #10022 is still is WIP to see if it can be refactored to benefit from the recent message container improvements.

luos pushed a commit to esl/rabbitmq-server that referenced this issue Dec 13, 2023
- Timestamps are milliseconds in AMQP 1.0, but in AMQP 0.9.1 it is seconds.
  Fixed by multiplying the timestamp by 1 000.
- Shovel crashed if user_id was set in the message because the encoding
  was as utf8 while it should be a byte array.
- Negative integers were encoded as integers - therefore leading to
  incorrect positive values.
- Float values were not supported by the client.
- Fixed priority header encoding in AMQP 1.0. It was set as uint but it
  should be ubyte.
- Priority of the message is now in the Headers instead of Application
  Properties. This is potentially a breaking change.

Fixes: rabbitmq#7508
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants