Skip to content

msgpack 1.0.0 changes default string encoding / breaks fluentbit communication #157

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
Closed
ptramsey opened this issue Feb 20, 2020 · 5 comments · Fixed by #158
Closed

msgpack 1.0.0 changes default string encoding / breaks fluentbit communication #157

ptramsey opened this issue Feb 20, 2020 · 5 comments · Fixed by #158

Comments

@ptramsey
Copy link

ptramsey commented Feb 20, 2020 •
edited
Loading

https://pypi.org/project/msgpack/1.0.0/ -- see the section, "Major breaking changes in 1.0":

use_bin_type=True by default. bytes are encoded in bin type in msgpack. If you are still sing Python 2, you must use unicode for all string types. You can use use_bin_type=False to encode into old msgpack format.

if use_bin_type=False isn't passed to FluentSender in msgpack_kwargs, fluentbit will see that the first element of each received message is not of type MSGPACK_OBJECT_STR, and refuse to enqueue the message. If you have debug output turned on in fluent-bit, you should see it output a message like,

[2020/02/20 21:19:55] [debug] [in_fw] parser: invalid tag format, skip.

I have verified that this happens in fluent-bit 1.3.5, and while I haven't tried it using 1.3.8, the relevant code in that release appears unchanged.
@ptramsey
Copy link
Author

I'm not sure what the correct path forward is --- perhaps use_bin_type=False should be included by default in msgpack_kwargs. Alternatively, the dependency could be pinned to python-msgpack <= 0.6.2, since 1.0.0 has degraded performance for python 2.

@arcivanov
Copy link
Member

I'm going to pin it under 1.0.0 for now as a short term solution. Python 2 being dead is not a terrible concern anymore.

Thanks for the report!

@arcivanov arcivanov mentioned this issue Feb 24, 2020
Merged
@keuko
Copy link

keuko commented Jan 15, 2021

I'm going to pin it under 1.0.0 for now as a short term solution. Python 2 being dead is not a terrible concern anymore.

Thanks for the report!

Hello,

Firstly I would like to say thank you for python-fluent-logger !
Now, please could I ask if there is a plan to support msgpack > 1.0.0 ? As we have plans in Kolla Openstack to integrate fluent-logger into central_logging of openstack. And newest lower-constraints in openstack is msgpack 1.0.2 ..

https://review.opendev.org/c/openstack/kolla-ansible/+/755775
https://review.opendev.org/c/openstack/kolla/+/759855

I am also packaging python-fluent-logger for debian distro, so can I expect new version ?
This issue was closed almost one year ago and from your comment pin msgpack to < 1.0.0 was only short solution..

Thank you very much !
Michal Arbet ( kevko )

@keuko
Copy link

keuko commented Jan 15, 2021

There is also pull request for unpin msgpack and some patch here ->
#171

Could you please verify and merge ?

@arcivanov arcivanov mentioned this issue Jan 26, 2021
Closed
@arcivanov
Copy link
Member

In #173 I'm removing Python 2 compatibility and setting msgpack>1.0 dependency. This way everyone on Python 2 still can use 0.9.x.

mcipperly pushed a commit to mcipperly/fluent-logger-python that referenced this issue Apr 21, 2021
@arcivanov @mcipperly
Pin msgpack below 1.0.0 to prevent API incompatibility
0a580c4 
fixes fluent#157

Signed-off-by: Matt Cipperly <mcipperly@fb.com>
mcipperly pushed a commit to mcipperly/fluent-logger-python that referenced this issue Apr 21, 2021
@arcivanov @mcipperly
Pin msgpack below 1.0.0 to prevent API incompatibility
3c31c18 
fixes fluent#157

Signed-off-by: Matt Cipperly <mcipperly@fb.com>
mcipperly pushed a commit to mcipperly/fluent-logger-python that referenced this issue Apr 21, 2021
@arcivanov @mcipperly
Pin msgpack below 1.0.0 to prevent API incompatibility
1ac881f 
fixes fluent#157
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Pin msgpack below 1.0.0 to prevent API incompatibility arcivanov/fluent-logger-python
3 participants
@ptramsey @arcivanov @keuko