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.

Sign up for GitHub

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

Jump to bottom

filebeat: add docker image name field to journald input #40450

Merged
merged 3 commits into from
Aug 30, 2024
Merged

filebeat: add docker image name field to journald input #40450

merged 3 commits into from
Aug 30, 2024

Conversation

msanft
Copy link
Contributor

@msanft msanft commented Aug 7, 2024

Proposed commit message

Since commit 5f7e102 in moby/moby, the IMAGE_NAME field is added to Docker log messages sent to journald. This was previously undocumented in Docker, which is changed by 69a143d. This adds a translation for the field to Filebeat.

Additionally, the documentation for the container.log.tag translation is fixed.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

@msanft
filebeat: add docker image name field to journald input
00b7f18 
Since commit [5f7e102](moby/moby@5f7e102) in moby/moby, the `IMAGE_NAME` field is added to Docker log messages sent to journald. This was previously undocumented in Docker, which is changed by [69a143d](docker/docs@69a143d). This adds a translation for the field to Filebeat.

Additionally, the documentation for the `container.log.tag` translation is fixed.

Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com>
@msanft msanft requested a review from a team as a code owner August 7, 2024 13:14
@msanft msanft requested review from rdner and leehinman August 7, 2024 13:14
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Aug 7, 2024
@cla-checker-service CLA Checker Service
Copy link

cla-checker-service bot commented Aug 7, 2024 •
edited
Loading

💚 CLA has been signed

@msanft
Copy link
Contributor Author

msanft commented Aug 7, 2024

The PR template asks to add labels, but I'm unable to do so.

@mergify mergify bot assigned msanft Aug 7, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Aug 7, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @msanft? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@pierrehilbert pierrehilbert added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Aug 7, 2024
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Aug 7, 2024
@pierrehilbert pierrehilbert requested a review from belimawr August 7, 2024 14:03
@belimawr belimawr added the backport-skip Skip notification from the automated backport with mergify label Aug 9, 2024
@belimawr
Copy link
Contributor

belimawr commented Aug 9, 2024

Thanks for the PR @msanft! It looks great!

I've just merged some changes to the file you edited, I'll merge main into your branch to check for any conflicts.

belimawr
belimawr requested changes Aug 9, 2024
View reviewed changes
Copy link
Contributor

@belimawr belimawr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changelog entry is missing, could you add one in CHANGELOG.next.asciidoc?

@msanft
filebeat: add docker image name field to journald input
307dce4 
Since commit [5f7e102](moby/moby@5f7e102) in moby/moby, the `IMAGE_NAME` field is added to Docker log messages sent to journald. This was previously undocumented in Docker, which is changed by [69a143d](docker/docs@69a143d). This adds a translation for the field to Filebeat.

Additionally, the documentation for the `container.log.tag` translation is fixed.

Signed-off-by: Moritz Sanft <58110325+msanft@users.noreply.github.com>
@msanft
Copy link
Contributor Author

msanft commented Aug 9, 2024

Thanks for the review @belimawr!
Added the relevant changelog entry. Feel free to correct me on wording here. I wasn't exactly sure how to put it best.

@msanft msanft requested a review from belimawr August 9, 2024 13:53
@pierrehilbert
Copy link
Collaborator

/test

@pierrehilbert
Copy link
Collaborator

run docs-build

pierrehilbert
pierrehilbert reviewed Aug 22, 2024
View reviewed changes
filebeat/docs/inputs/input-journald.asciidoc
@@ -284,7 +284,8 @@ available:
`CONTAINER_ID_FULL`:: `container.id`
`CONTAINER_NAME`:: `container.name`
`CONTAINER_PARTIAL_MESSAGE`:: `container.partial`
`CONTAINER_TAG`:: `container.image.tag`
`CONTAINER_TAG`:: `container.log.tag`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be sure to understand, what is the reason to switch from container.image.tag to container.log.tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a matter of correcting the docs. The actual field is called container.log.tag

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense thanks for the explanation!
Looks good to be merged then!

@pierrehilbert pierrehilbert merged commit 59612ac into elastic:main Aug 30, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pierrehilbert pierrehilbert pierrehilbert left review comments

@leehinman leehinman leehinman approved these changes

@belimawr belimawr belimawr approved these changes

@rdner rdner Awaiting requested review from rdner rdner is a code owner automatically assigned from elastic/elastic-agent-data-plane

Assignees

@msanft msanft

Labels
backport-skip Skip notification from the automated backport with mergify Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@msanft @elasticmachine @belimawr @pierrehilbert @leehinman