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

cfl_record_accessor: cfl_ra_key: Implement generic CFL based record accessor #9566

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cosmo0920
Copy link
Contributor

Currently, we didn't implement CFL based record accessor. Which is also called as cobj.
This leads that we're not able to access nested cobjets in processor plugins.
To resolve this, I implemented cobj based record accessor and that infrastructure, cobj_ra_key.

In cobj based record accessor, the same mechanism of parser for record accessor syntax is able to use.
So, I didn't implement cobj specific parser of that syntax.
The main difference of the accessor is creating cobj based record accessor related sturct and use it there.
Other part is mostly copy and paste and then adjust for the operation of cobj.


Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

The written down test cases are not causing memory leaks:

$ valgrind bin/flb-it-cobj_record_accessor -v
<snip>
==878874== 
==878874== HEAP SUMMARY:
==878874==     in use at exit: 0 bytes in 0 blocks
==878874==   total heap usage: 16,823 allocs, 16,823 frees, 1,681,429 bytes allocated
==878874== 
==878874== All heap blocks were freed -- no leaks are possible
==878874== 
==878874== For lists of detected and suppressed errors, rerun with: -s
==878874== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • Run local packaging test showing all targets (including any new ones) build.
  • Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

@cosmo0920 cosmo0920 force-pushed the cosmo0920-implement-cobj-based-record-accessor branch from 1ef920c to 1cdce4d Compare November 8, 2024 05:01
@cosmo0920 cosmo0920 marked this pull request as ready for review November 8, 2024 09:22
@cosmo0920 cosmo0920 added this to the Fluent Bit Next milestone Nov 12, 2024
@edsiper
Copy link
Member

edsiper commented Nov 13, 2024

Due to the use cases we have, where record accessor patterns will be used go beyond logs, it will apply to metrics and traces, for hence I think the following changes are necessary:

  • cobj is only about logs, however record accessor internally operates on top of a CFL data structure. cobj is just a wrapper
  • we need to have record accessor API for other signal types such as metrics and traces

The change proposed, is to have a flb_cfl_record_accessor(struct cfl_variant *var) or flb_cfl_record_accessor(struct cfl_kvlist *kvist) so it can be used for any component inside Fluent Bit.

@cosmo0920 cosmo0920 force-pushed the cosmo0920-implement-cobj-based-record-accessor branch from 1cdce4d to 110c1bb Compare November 14, 2024 09:00
@cosmo0920 cosmo0920 changed the title cobj_record_accessor: cobj_ra_key: Implement cobj based record accessor cfl_record_accessor: cfl_ra_key: Implement generic CFL based record accessor Nov 14, 2024
@cosmo0920 cosmo0920 force-pushed the cosmo0920-implement-cobj-based-record-accessor branch from 110c1bb to 6ec10b9 Compare November 14, 2024 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants