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

Serialize record timestamp as msgpack Timestamp #6

Merged
merged 4 commits into from
Aug 14, 2024

Conversation

DoumanAsh
Copy link
Owner

This changes serialization to msgpack extension type for Timestamp encoding
This should guarantee interpretation to be correct

Closes #5

@DoumanAsh
Copy link
Owner Author

It seems fluentd is hardcoded to treat it as integer, need to investigate if it can be changed

@JoseCVM
Copy link

JoseCVM commented Aug 13, 2024

Yes, it didn't work here with an error (this specific instance is after I messed with the format but same issue, seems to expect a number):

2024-08-13 02:03:26.790600262 +0000 fluent.error: {"host":"172.17.0.1","port":52048,"error":"#<NoMethodError: undefined method `to_i' for [-1, [28, 212, 187, 48, 102, 186, 189, 18]]:Array>","message":"unexpected error on reading data host=\"172.17.0.1\" port=52048 error_class=NoMethodError error=\"undefined method `to_i' for [-1, [28, 212, 187, 48, 102, 186, 189, 18]]:Array\""}

I was trying to see if fitting it to the format described in forward protocol specification would help, my understanding is that this is not the format fluent expects to ingest time but is for plugin writers, but was the best info I could find for accepted time formats, but I couldn't get it to work

@DoumanAsh
Copy link
Owner Author

DoumanAsh commented Aug 13, 2024

That's good one, we can try to encode it this way and see if it works
I will give it a go later

UPD: couldn't make it work for now too

@DoumanAsh
Copy link
Owner Author

@JoseCVM I figured out how to serialize timestamp as per spec

@DoumanAsh
Copy link
Owner Author

DoumanAsh commented Aug 13, 2024

Looking at source code, we can actually send float it seems
https://github.com/fluent/fluentd/blob/master/lib/fluent/time.rb#L336

This might be better due to having higher size

@DoumanAsh DoumanAsh merged commit d0fb7af into master Aug 14, 2024
1 check passed
@DoumanAsh DoumanAsh deleted the msgpack_timestamp branch August 14, 2024 11:45
@DoumanAsh
Copy link
Owner Author

@JoseCVM I released 0.4.1 with optional feature event_time to enable encoding timestamp with nanosecond part

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.

2 participants