-
Couldn't load subscription status.
- Fork 77
feat: Update Mongodb receiver to use LoggingReceiverMacro
#2003
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
base: master
Are you sure you want to change the base?
Conversation
| logging.googleapis.com/instrumentation_source: agent.googleapis.com/mongodb | ||
| logName: projects/my-project/logs/transformation_test | ||
| severity: 200.0 | ||
| timestamp: now |
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.
Since all the log entries here seem to have timestamp: now (which happens when the log didn't parse a specific timestamp), there may be a bug in the timestamp parsing.
It may predate this PR, but or it could be a mismatched format. Please address this missing timestamp.
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.
In the original implementation there was some special logic to grab a specific fluentbit component from the LoggingProcessorParseRegix at a specific index which was just the time parsing section. Because this is now becoming more generic we can not just grab a given index.
I created a new parser that is just made for parsing the timestamp.
This all stems from the following comment where the nested timestamp has to be brought to the top level of the JSON structure.
// have to bring $date to top level in order for it to be parsed as timeKey
// see https://github.com/fluent/fluent-bit/issues/1013
I am open to other solutions here but this seems like a reasonable solution that works similarly to the old implementation.
apps/mongodb.go
Outdated
|
|
||
| func init() { | ||
| confgenerator.LoggingReceiverTypes.RegisterType(func() confgenerator.LoggingReceiver { return &LoggingReceiverMongodb{} }) | ||
| confgenerator.RegisterLoggingReceiverMacro(func() LoggingReceiverMacroMongodb { |
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.
Please register using RegisterLoggingFilesProcessorMacro instead. There is no need to only register the receiver anymore.
| Field string | ||
| } | ||
|
|
||
| func (p LoggingProcessorRemoveField) Components(ctx context.Context, tag, uid string) []fluentbit.Component { |
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.
I've thought about this a bit and realized that the operations done by LoggingProcessorRemoveField, LoggingProcessorHardRename and LoggingProcessorNestLift can be replicated with LoggingProcessorModifyFields. Please remove this processors and using modify fields instead.
Here is a list of this operations :
LoggingProcessorRemoveFieldcan be replicated with :
confgenerator.LoggingProcessorModifyFields{
Fields: map[string]*confgenerator.ModifyField{
"jsonPayload.Field": {
MoveFrom: "jsonPayload.Field",
OmitIf: `jsonPayload.Field =~ ".*"`
},
},
}
LoggingProcessorHardRenamecan be replicated with :
confgenerator.LoggingProcessorModifyFields{
Fields: map[string]*confgenerator.ModifyField{
"jsonPayload.dest": {
MoveFrom: "jsonPayload.src",
},
},
}
LoggingProcessorNestLiftcan be replicated with :
confgenerator.LoggingProcessorModifyFields{
Fields: map[string]*confgenerator.ModifyField{
"jsonPayload.prefix_Field": {
MoveFrom: "jsonPayload.nested.Field",
},
},
}
Using LoggingProcessorModifyFields instead of creating processors improves a lot the Mongodb processor the following ways :
- Removes tech debt of having to maintain custom
fluent-bitconfigs. - There is no need to implement each of this processors with Otel (modify fields already has both implementations).
- Improves readability of the Mongodb processor.
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.
Most of the solutions are valid here except there is one condition where LoggingProcessorHardRename is needed due to side effects of MoveFrom.
When dealing with a log from wiredTiger we want to overwrite the top level msg field with the nested attr.message field. We only want to do this if the attr.message field is present. When using MoveFrom it will always overwrite the field with an empty object when attr.message is not present.
From my understanding, I can't think of a solution that allows us to preserve the old msg if attr.message is not present with the current functionality.
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.
I think we can use the OmitIf feature for this situation. Here the condition jsonPayload.attr.message :* checks if the field attr.message is present in the log. AFAIU, using "OmitIf: NOT jsonPayload.attr.message :*" will skip the MoveFrom if the field is not present.
confgenerator.LoggingProcessorModifyFields{
Fields: map[string]*confgenerator.ModifyField{
"jsonPayload.message": {
MoveFrom: "jsonPayload..attr.message",
OmitIf: `NOT jsonPayload.attr.message :*`,
},
},
}
Note : I'm partially guiding myself with the 1 modify_fields public documentation, so this may not be fully correct, but i think if this is possible to be done with ModifyFields it would use OmitIf in some capacity. We can also explore the ModifyFields implementation to get more clarity.
Footnotes
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.
I tried that and the OmitIf functionality seems to actually omit the destination field if it matches.
From the docs you linked
If the filter matches the input log record, the output field will be unset.
so we end up deleting the jsonPayload.message field entirely if we don't have the nested message
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.
Ohhh! Ok! 🤔 Please try further with other variations of the OmitIf to see if there is way to make it work.
Alternative
If this doesn't work, we can try a solution that uses a custom LUA function (or keep using HardRename) for fluent-bit and a ModifyFields + CustomConverFunc + OTTL solution for Otel .
I outlined a similar idea here #2014 (comment) . In summary you would create a struct ConditionalMoveFromProcessor which implements both InternalLoggingProcessor (implements Components() for fluent-bit) and InternalOtelProcessor (implements Processors() for otel) and return the custom logic for each subagent.
You can probably also reuse LoggingProcessorHardRename (though i rather have a more specific name) and implement a Processors() method that uses ModifyFields + CustomConverFunc + OTTL.
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.
I was not able to come up with anything that worked with OmitIf and updated the name of HardRename to something more clear as well as adding the otel Processors() logic.
Will wait for @quentinmit to see if he has a better solution
apps/mongodb.go
Outdated
| type LoggingReceiverMongodb struct { | ||
| LoggingProcessorMongodb `yaml:",inline"` | ||
| ReceiverMixin confgenerator.LoggingReceiverFilesMixin `yaml:",inline" validate:"structonly"` | ||
| type LoggingReceiverMacroMongodb struct { |
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.
After registering with RegisterLoggingFilesProcessorMacro there is no need to have a LoggingReceiverMacroMongodb.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Description
Refactored the Mongodb receiver to use
LoggingReceiverMacroA few issues came up with this refactor that I will need some guidance on:
ElasticsearchJsonreceiver where we just register the processor in thetransformation_test.gofileRelated issue
How has this been tested?
Validated there were no changes to the generated fluentbit golden file before and after the refactor
Checklist: