Populate ecs fields by pod and container metricsets #30181

MichaelKatsoulis · 2022-02-03T12:03:11Z

What does this PR do?

After new container ECS fields get GA (elastic/ecs#1747) kubernetes module need to start populating those fields.

The new fields are

container.cpu.usage
container.memory.usage
container.network.egress.bytes
container.network.ingress.bytes

These fields map to kubernetes fields:

kubernetes.container.cpu.usage.node.pct by container metricset
kubernetes.container.memory.usage.node.pct by container metricset
kubernetes.pod.network.tx.bytes by pod metricset
kubernetes.pod.network.rx.bytes by pod metricset

respectively.

Why is it important?

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.

How to test this PR locally

Related issues

Screenshots

mergify · 2022-02-03T12:03:27Z

This pull request does not have a backport label. Could you fix it @MichaelKatsoulis? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

elasticmachine · 2022-02-03T13:37:14Z

💚 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

Start Time: 2022-03-29T09:46:14.435+0000
Duration: 52 min 39 sec

Test stats 🧪

Test	Results
Failed	0
Passed	3517
Skipped	877
Total	4394

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

ChrsMark · 2022-02-04T08:39:49Z

metricbeat/module/kubernetes/container/container.go

+		// Enrich event with container ECS fields
+		containerEcsFields := ecsfields(event)
+		if len(containerEcsFields) != 0 {
+			e.RootFields = common.MapStr{


This will override any existing RootFields right? event variable will already have RootFields coming from

beats/metricbeat/module/kubernetes/util/kubernetes.go

Line 495 in c1d4f28

e.RootFields = metaFieldsMapStr

I added a check in 0a0a543

MichaelKatsoulis · 2022-02-07T10:42:36Z

@ChrsMark I added some units tests as well

ChrsMark

I wonder if we should wait until elastic/ecs#1747 is settled down before moving on with this. wdyt?

MichaelKatsoulis · 2022-02-08T08:44:44Z

I wonder if we should wait until elastic/ecs#1747 is settled down before moving on with this. wdyt?

Yes. I will move it as blocked and wait.

…_ecs_fields

tetianakravchenko · 2022-03-28T18:05:32Z

metricbeat/module/kubernetes/container/data.go

@@ -114,28 +114,28 @@ func eventMapping(content []byte, perfMetrics *util.PerfMetricsCache) ([]common.
 			}

 			if container.StartTime != "" {
-				containerEvent.Put("start_time", container.StartTime)
+				_, _ = containerEvent.Put("start_time", container.StartTime)


should we maybe add proper handling of errors here, as a debug messages?
not in this PR, but at least create a ticket for that

ChrsMark

Left some comments.

metricbeat/module/kubernetes/pod/pod.go

metricbeat/module/kubernetes/container/container.go

ChrsMark · 2022-03-29T08:06:40Z

metricbeat/module/kubernetes/container/container.go

+		containerEcsFields := ecsfields(event)
+		if len(containerEcsFields) != 0 {
+			if e.RootFields != nil {
+				e.RootFields.DeepUpdate(common.MapStr{


RootFields can already contain container key here so this is why using DeepUpdate right? Can you also add a comment about this so as to prevent any unwanted changes in the future if someone oversee the fact of the preexisting container key?

ChrsMark · 2022-03-29T08:07:21Z

metricbeat/module/kubernetes/container/data.go

@@ -114,28 +114,28 @@ func eventMapping(content []byte, perfMetrics *util.PerfMetricsCache) ([]common.
 			}

 			if container.StartTime != "" {
-				containerEvent.Put("start_time", container.StartTime)
+				_, _ = containerEvent.Put("start_time", container.StartTime)


ChrsMark · 2022-03-29T08:08:01Z

metricbeat/module/kubernetes/container/data.go

 			}

 			if nodeCores > 0 {
-				containerEvent.Put("cpu.usage.node.pct", float64(container.CPU.UsageNanoCores)/1e9/nodeCores)
+				_, _ = containerEvent.Put("cpu.usage.node.pct", float64(container.CPU.UsageNanoCores)/1e9/nodeCores)


Too many unhandled errors. Wondering if we should actually handle them properly.

We don't want to do anything specific here. Just continue to the next fields

Still don't like this format with unhandled errors but if you feel it's ok you can ship it.

I don't either. It is like this in most of the beats modules. If we want to handle it somehow(log something), let's do it in subsequent pr, so we can make the FF for now

ChrsMark

Let's see how we can improve the code style in follow-ups.

* Populate ecs fields by pod and container metricsets * Update changelog

Populate ecs fields by pod and container metricsets

4ca4e6c

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Feb 3, 2022

MichaelKatsoulis self-assigned this Feb 3, 2022

MichaelKatsoulis marked this pull request as draft February 3, 2022 12:03

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Feb 3, 2022

MichaelKatsoulis added Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team and removed backport-skip Skip notification from the automated backport with mergify labels Feb 3, 2022

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Feb 3, 2022

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Feb 3, 2022

MichaelKatsoulis requested a review from ChrsMark February 3, 2022 15:14

ChrsMark requested changes Feb 4, 2022

View reviewed changes

MichaelKatsoulis added 2 commits February 4, 2022 12:22

Check existance of rootfields

0a0a543

Add unit tests

ceff8f8

ChrsMark reviewed Feb 7, 2022

View reviewed changes

MichaelKatsoulis mentioned this pull request Feb 10, 2022

Add new ecs fields in kubernetes integration elastic/integrations#2666

Closed

Merge remote-tracking branch 'upstream/main' into kubernetes_populate…

c05e5e8

…_ecs_fields

This was referenced Mar 22, 2022

Add container fields to metricbeat #30957

Closed

Containerd populate ecs fields #30267

Closed

MichaelKatsoulis added 3 commits March 28, 2022 18:46

Merge remote-tracking branch 'upstream/main' into kubernetes_populate…

f6c16ca

…_ecs_fields

golangci-lint errors fix

37558bb

lint updates

11eb986

MichaelKatsoulis marked this pull request as ready for review March 28, 2022 16:59

strings cannot be capitalized

64ffde8

tetianakravchenko approved these changes Mar 28, 2022

View reviewed changes

MichaelKatsoulis requested a review from tetianakravchenko March 29, 2022 08:00

ChrsMark reviewed Mar 29, 2022

View reviewed changes

ChrsMark approved these changes Mar 29, 2022

View reviewed changes

Update changelog

e053a66

MichaelKatsoulis requested review from a team as code owners March 29, 2022 09:45

MichaelKatsoulis removed request for a team March 29, 2022 09:46

MichaelKatsoulis merged commit 9e0c0fb into elastic:main Mar 29, 2022

MichaelKatsoulis mentioned this pull request Mar 30, 2022

Improve error handling in Kubernetes module #31067

Closed

emilioalvap pushed a commit to emilioalvap/beats that referenced this pull request Apr 6, 2022

Populate ecs fields by pod and container metricsets (elastic#30181)

38c52f7

* Populate ecs fields by pod and container metricsets * Update changelog

kush-elastic pushed a commit to kush-elastic/beats that referenced this pull request May 2, 2022

Populate ecs fields by pod and container metricsets (elastic#30181)

fc9e35d

* Populate ecs fields by pod and container metricsets * Update changelog

chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023

Populate ecs fields by pod and container metricsets (#30181)

13a072f

* Populate ecs fields by pod and container metricsets * Update changelog

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Populate ecs fields by pod and container metricsets #30181

Populate ecs fields by pod and container metricsets #30181

MichaelKatsoulis commented Feb 3, 2022 •

edited

Loading

mergify bot commented Feb 3, 2022

elasticmachine commented Feb 3, 2022 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

ChrsMark Feb 4, 2022

MichaelKatsoulis Feb 4, 2022

MichaelKatsoulis commented Feb 7, 2022

ChrsMark left a comment •

edited

Loading

MichaelKatsoulis commented Feb 8, 2022

tetianakravchenko Mar 28, 2022

ChrsMark Mar 29, 2022

ChrsMark left a comment

ChrsMark Mar 29, 2022

ChrsMark Mar 29, 2022

ChrsMark Mar 29, 2022

MichaelKatsoulis Mar 29, 2022

ChrsMark Mar 29, 2022

MichaelKatsoulis Mar 29, 2022

ChrsMark left a comment

Populate ecs fields by pod and container metricsets #30181

Populate ecs fields by pod and container metricsets #30181

Conversation

MichaelKatsoulis commented Feb 3, 2022 • edited Loading

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Related issues

Screenshots

mergify bot commented Feb 3, 2022

elasticmachine commented Feb 3, 2022 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelKatsoulis commented Feb 7, 2022

ChrsMark left a comment • edited Loading

Choose a reason for hiding this comment

MichaelKatsoulis commented Feb 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrsMark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrsMark left a comment

Choose a reason for hiding this comment

MichaelKatsoulis commented Feb 3, 2022 •

edited

Loading

elasticmachine commented Feb 3, 2022 •

edited by jenkins-beats-ci bot

Loading

ChrsMark left a comment •

edited

Loading