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

filter_ecs: Retrieve container ID from record field #9033

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

Conversation

RaJiska
Copy link

@RaJiska RaJiska commented Jul 2, 2024

Adds container_id_field_name to retrieve the container ID instead of deducing it from the tags via ecs_tag_prefix which is too restrictive to use as too invasive over the logic the user wants to use.

Maintained the retrieval of the container ID to not introduce breaking change, but using container_id_field_name will override and retrieve container ID from the specified field.

Fixes the following issue: #9028


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:

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

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

Documentation

Backporting

  • [N/A] 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.

@RaJiska RaJiska force-pushed the filter-ecs-container-id-field branch from 85c937c to b4ba446 Compare July 2, 2024 15:57
Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
@edsiper
Copy link
Member

edsiper commented Jul 5, 2024

Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
@RaJiska RaJiska force-pushed the filter-ecs-container-id-field branch from da51c32 to 59cd20b Compare July 6, 2024 08:50
@RaJiska
Copy link
Author

RaJiska commented Jul 6, 2024

@edsiper Added format checks as well as well as an extra test for invalid data.

@RaJiska
Copy link
Author

RaJiska commented Jul 16, 2024

@edsiper One of the test fails, though it doesn't seem related to the changes I made, is this a random failure or was some kind of side-effect introduced? I had all passing when running locally.

@RaJiska
Copy link
Author

RaJiska commented Aug 1, 2024

Bump @edsiper @PettitWesley

@RaJiska
Copy link
Author

RaJiska commented Aug 26, 2024

Bump @edsiper @PettitWesley 👀

@RaJiska
Copy link
Author

RaJiska commented Sep 30, 2024

@edsiper @PettitWesley

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.

2 participants