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.

Sign up for GitHub

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

Jump to bottom

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 · May be fixed by #11729
Closed

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

Haigutus opened this issue Mar 2, 2023 · 5 comments · May be fixed by #11729
Labels
rabbitmq-shovel
Milestone
3.12.11

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

rabbitmq-server/deps/rabbitmq_shovel/src/rabbit_amqp10_shovel.erl

Line 397 in 942c4ab

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 •
edited
Loading

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
@luos
Convert AMQP 1.0 props and app props to AMQP 0.9.1 props and headers
2c5b2ff 
- 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 luos mentioned this issue Dec 1, 2023
Closed
12 tasks
olikasg added a commit to esl/rabbitmq-server that referenced this issue Dec 4, 2023
@olikasg
Convert AMQP 1.0 props and app props to AMQP 0.9.1 props and headers
fbc51d6 
- 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
@olikasg @luos
Convert AMQP 1.0 props and app props to AMQP 0.9.1 props and headers
b1ba0b9 
- 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
@olikasg @luos
Convert AMQP 1.0 props and app props to AMQP 0.9.1 props and headers
21c37b3 
- 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
@olikasg @luos
Convert AMQP 1.0 props and app props to AMQP 0.9.1 props and headers
8e954ff 
- 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 luos mentioned this issue Dec 4, 2023
Merged
12 tasks
@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
@olikasg @luos
Convert AMQP 1.0 props and app props to AMQP 0.9.1 props and headers
4bdba10 
- 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 mentioned this issue Jan 18, 2024
Closed
mergify bot pushed a commit that referenced this issue Jul 16, 2024
@olikasg @mergify
Convert AMQP 1.0 props and app props to AMQP 0.9.1 props and headers
9df76ec 
- 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: #7508
(cherry picked from commit 8e954ff)

# Conflicts:
#	deps/rabbitmq_shovel/src/rabbit_amqp10_shovel.erl
@mergify mergify bot mentioned this issue Jul 16, 2024
Merged
12 tasks
kjnilsson pushed a commit that referenced this issue Jul 16, 2024
@olikasg @kjnilsson
Convert AMQP 1.0 props and app props to AMQP 0.9.1 props and headers
b729106 
- 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: #7508
(cherry picked from commit 8e954ff)
mergify bot pushed a commit that referenced this issue Jul 16, 2024
@olikasg @mergify
Convert AMQP 1.0 props and app props to AMQP 0.9.1 props and headers
78ec03a 
- 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: #7508
(cherry picked from commit 8e954ff)

# Conflicts:
#	deps/amqp10_client/src/amqp10_msg.erl
#	deps/rabbitmq_shovel/src/rabbit_amqp10_shovel.erl
@mergify mergify bot mentioned this issue Jul 16, 2024
Open
12 tasks
kjnilsson pushed a commit that referenced this issue Jul 16, 2024
@olikasg @kjnilsson
Convert AMQP 1.0 props and app props to AMQP 0.9.1 props and headers
f5b1f4e 
- 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: #7508
(cherry picked from commit 8e954ff)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
rabbitmq-shovel
Projects
None yet
Milestone
3.12.11
3 participants
@michaelklishin @Haigutus @ikavgo