Use alias to report container image in k8s parts by ChrsMark · Pull Request #24380 · elastic/beats

ChrsMark · 2021-03-05T10:11:48Z

What does this PR do?

This PR leverages type :alias for kubernetes.container.image so as to copy the fields under ECS container.image.name.
This affects metadata collected by kubernetes autodiscover provider, add_kubernetes_metadata processor and state_container metricset.

Why is it important?

In order to have a persistent way to report ECS container fields, without the need to duplicate fields and store both which will impact ECS.

How to test this PR locally

Before all test verify that fields.yml on your testing env is properly updated with the alias change and manually setup and verify that mappings are properly loaded in ES.
Check that autodiscover meta work:

filebeat.autodiscover:
      providers:
        - type: kubernetes
          node: ${NODE_NAME}
          hints.enabled: true
          hints.default_config:
            type: container
            paths:
              - /var/log/containers/*${data.kubernetes.container.id}.log

Make sure that container.image.name exist in the final documents in ES and you able to search on based on it while kubernetes.container.image is not populated in the event but is searchable because of the alias.
3. Check that add_kubernetes_metadata processor works:

filebeat.inputs:
    - type: container
      paths:
        - /var/log/containers/*.log
      processors:
        - add_kubernetes_metadata:
            host: ${NODE_NAME}
            indexers:
            - container:
            matchers:
            - logs_path:
                logs_path: "/var/log/containers/"

Make sure that container.image.name exist in the final documents in ES and you able to search on based on it while kubernetes.container.image is not populated in the event but is searchable because of the alias.

Verify with state_container metricset enabled that, container.image.name exist in the final documents in ES and you able to search based on it while kubernetes.container.image is not populated in the event but is searchable because of the alias.

Related issues

Screenshots

Signed-off-by: chrismark <chrismarkou92@gmail.com>

elasticmachine · 2021-03-05T10:11:51Z

Pinging @elastic/integrations (Team:Integrations)

libbeat/processors/add_kubernetes_metadata/_meta/fields.yml

elasticmachine · 2021-03-05T12:11:58Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Build Cause: Pull request #24380 updated
Start Time: 2021-03-10T10:38:51.383+0000
Duration: 73 min 26 sec
Commit: 67a4d9b

Test stats 🧪

Test	Results
Failed	0
Passed	45249
Skipped	4972
Total	50221

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	45249
Skipped	4972
Total	50221

Signed-off-by: chrismark <chrismarkou92@gmail.com>

libbeat/autodiscover/providers/kubernetes/pod.go

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark · 2021-03-09T13:53:03Z

@jsoriano @kaiyan-sheng could you please take another look please? #24380 (comment) seems to be valid since code should set the values under ECS fields and not under kubernetes.container.*. kubernetes.container.* will be populated through the alias. I also added testing notes and some screenshots.

kaiyan-sheng

Change looks good to me, do you think it's worth a changelog?

ChrsMark · 2021-03-09T15:32:55Z

Change looks good to me, do you think it's worth a changelog?

Well if this change is correct it should be transparent to the end users. Otherwise it's a breaking change 😬 ?

jsoriano

A couple of minor comments but it looks good.

jsoriano · 2021-03-09T15:51:39Z

metricbeat/module/kubernetes/state_container/state_container.go

+		// remove ECS container fields from kubernetes.container.* since they will be set through alias
+		event.Delete("image")
+		event.Delete("name")


Move these deletions to the previous ifs where it is checked if these fields exist?

jsoriano · 2021-03-09T15:58:59Z

libbeat/autodiscover/providers/kubernetes/pod.go

+				//Actual metadata that will enrich the event
 				"meta": common.MapStr{
-					"kubernetes": meta,
+					"kubernetes": meta, // these will be moved to ECS (container.name and container.image.name) through alias type


This comment looks a bit confusing to me. Not all these kubernetes.* meta fields are moved, right?

jsoriano · 2021-03-09T16:09:25Z

Change looks good to me, do you think it's worth a changelog?

Well if this change is correct it should be transparent to the end users. Otherwise it's a breaking change 😬 ?

I think that it can be a good idea to add a changelog entry for this change, in the breaking changes section, even if it should be transparent for most users. Even if not recommended, there can be users using custom mappings or no mapping at all and this change can be breaking for them. The entry could say something as "kubernetes.container.name and kubernetes.container.image fields are now aliases to their ECS equivalents".

Btw, it could be also breaking in the Metrics UI if it uses the old kubernetes fields and it doesn't handle aliases. We should check this, preferably before merging. @ChrsMark could you confirm this?

jsoriano · 2021-03-09T16:21:44Z

metricbeat/module/kubernetes/state_container/_meta/data.json

+    "image": {
+      "name": "ubuntu:latest"
+    },
+    "name": "ubuntu"


Sorry, I have just thought on something that can be important. kubernetes.container.name and container.name are actually not the same thing, and maybe this was the reason why this field was not filled here.
kubernetes.container.name is the name of a container in its pod, this is a kubernetes-specific concept and can make sense in a kubernetes-specific field.
container.name would be the container name in the runtime, that will be probably different, but would be the same concept in multiple orchestrators using the same container runtime.

On the other hand, the image reported by kubernetes.container.image is the image in the runtime, so it is probably ok if this is moved to a common field.

…name Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark · 2021-03-10T10:35:44Z

Long story short, kubernates.container.name field should not be the same as container.name field since the former is the container name as defined in Pod's spec while the later is the (actual) name of the container as reported by the runtime.

In this, this PR will only apply the change on kubernetes.image.name -> container.image.name.

I updated the description with accordingly (with new screenshots). @kaiyan-sheng @jsoriano sorry for the back-and-forth on this.

Signed-off-by: chrismark <chrismarkou92@gmail.com>

jsoriano

Thanks!

(cherry picked from commit 85066cb)

Use alias to report container name and image in k8s parts

41e6030

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark added containers Related to containers use case Team:Integrations Label for the Integrations team v7.13.0 kubernetes Enable builds in the CI for kubernetes labels Mar 5, 2021

ChrsMark self-assigned this Mar 5, 2021

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Mar 5, 2021

kaiyan-sheng reviewed Mar 5, 2021

View reviewed changes

libbeat/processors/add_kubernetes_metadata/_meta/fields.yml Outdated Show resolved Hide resolved

make update

912264d

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark requested a review from a team as a code owner March 5, 2021 12:50

jsoriano reviewed Mar 5, 2021

View reviewed changes

libbeat/autodiscover/providers/kubernetes/pod.go Show resolved Hide resolved

ChrsMark added 3 commits March 9, 2021 11:17

fix state_container tests

4af8f3c

Signed-off-by: chrismark <chrismarkou92@gmail.com>

fix autodiscover tests

bdb2e53

Signed-off-by: chrismark <chrismarkou92@gmail.com>

fix schema

e11ea81

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark requested review from jsoriano and kaiyan-sheng March 9, 2021 13:51

ChrsMark added the test-plan Add this PR to be manual test plan label Mar 9, 2021

kaiyan-sheng approved these changes Mar 9, 2021

View reviewed changes

jsoriano reviewed Mar 9, 2021

View reviewed changes

Revert change about container.name and keep only the one about image.…

95c5ae9

…name Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark changed the title ~~Use alias to report container name and image in k8s parts~~ Use alias to report container image in k8s parts Mar 10, 2021

Add changelog

67a4d9b

Signed-off-by: chrismark <chrismarkou92@gmail.com>

jsoriano approved these changes Mar 10, 2021

View reviewed changes

ChrsMark merged commit 85066cb into elastic:master Mar 10, 2021

This was referenced Mar 10, 2021

Verify ECS alignment in container fields reported by kubernetes module and meta processor #23585

Closed

Cherry-pick #24380 to 7.x: Use alias to report container image in k8s parts #24471

Merged

ChrsMark added a commit to ChrsMark/beats that referenced this pull request Mar 10, 2021

Use alias to report container image in k8s parts (elastic#24380)

9198581

(cherry picked from commit 85066cb)

ChrsMark added a commit that referenced this pull request Mar 10, 2021

Use alias to report container image in k8s parts (#24380) (#24471)

eeb4a21

(cherry picked from commit 85066cb)

andresrc added the test-plan-added This PR has been added to the test plan label Apr 22, 2021

ChrsMark mentioned this pull request May 25, 2021

docs: Prepare Changelog for 7.13.0 #25823

Merged

tetianakravchenko mentioned this pull request Jan 2, 2024

[Metricbeat][Kubernetes] Add container.name field to state_container #37515

Closed

6 tasks

Conversation

ChrsMark commented Mar 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

How to test this PR locally

Related issues

Screenshots

Uh oh!

elasticmachine commented Mar 5, 2021

Uh oh!

Uh oh!

elasticmachine commented Mar 5, 2021 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

💚 Flaky test report

Test stats 🧪

Uh oh!

Uh oh!

ChrsMark commented Mar 9, 2021

Uh oh!

kaiyan-sheng left a comment

Choose a reason for hiding this comment

Uh oh!

ChrsMark commented Mar 9, 2021

Uh oh!

jsoriano left a comment

Choose a reason for hiding this comment

Uh oh!

jsoriano Mar 9, 2021

Choose a reason for hiding this comment

Uh oh!

jsoriano Mar 9, 2021

Choose a reason for hiding this comment

Uh oh!

jsoriano commented Mar 9, 2021

Uh oh!

jsoriano Mar 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsoriano Mar 9, 2021

Choose a reason for hiding this comment

Uh oh!

ChrsMark commented Mar 10, 2021

Uh oh!

jsoriano left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ChrsMark commented Mar 5, 2021 •

edited

Loading

elasticmachine commented Mar 5, 2021 •

edited by jenkins-beats-ci bot

Loading

jsoriano Mar 9, 2021 •

edited

Loading