-
Notifications
You must be signed in to change notification settings - Fork 77
Add ActiveSupport Supports #112
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
Conversation
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
49c1ebc to
06089ee
Compare
daipom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Since the Active Support topic suddenly appears, it’s a bit hard to follow, so I’d like to clarify the purpose of this PR.
My understanding is that #111 is not directly related to Active Support itself, but it would be more convenient to align with the JSON format definition provided by Active Support.
This improvement would be useful. Looks good to me.
However, it seems a part of #111 is still unresolved.
I’m particularly concerned about how the timestamps of the Flunetd Event itself are handled.
In particular, when using nanosecond_precision, I’m concerned about what happens to the milliseconds of the Event’s timestamp if a JSON fallback occurs.
fluent-logger-ruby/lib/fluent/logger/fluent_logger.rb
Lines 49 to 51 in 6bcebac
| def to_json(*args) | |
| @sec.to_s | |
| end |
It seems that #111 should remain open.
|
|
||
| def as_json(*args) | ||
| @sec | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we should leave a comment on why we need this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment.
ec11c6d
The title change has made it much clearer. Thanks! |
Even with nanosecond_precision enabled, EventTime's nsec values seem to be truncated when JSON fallback occurs. |
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
The primary purpose of this PR is to respect the object's own to_json implementation instead of forcing JSON.generate. This allows any Ruby object (not just in Rails) to control its own JSON serialization format. Active Support is just the most prominent use case where to_json behavior differs significantly from JSON.generate. I will update the description to clarify that the main goal is generic to_json support, with Active Support cited as a key example. |
|
Thanks! |
Related to #111
Problem
Currently, the code uses
JSON.generate(msg)in JSON fallback. This forces a standard JSON serialization, potentially ignoring custom serialization logic defined within the objects themselves.Solution
Change the implementation to use
msg.to_json.JSON.parse(msg.to_json).to_msgpackThis allows objects to control their own string representation via their
to_jsonmethod before being parsed back into a Hash for MessagePack conversion.Context & Benefits
This change ensures that if an object has a custom
to_jsonimplementation, it is respected.Example: Active Support (Rails)
A major beneficiary of this change is environments where Active Support is loaded.
In Active Support,
to_jsondelegates toas_json, which handles:By using
msg.to_json, we ensure that logs forwarded by fluent-logger-ruby match the expected JSON format defined by the application/framework, rather than the raw internal representation.