Kubernetes metadata enhancements by ChrsMark · Pull Request #22189 · elastic/beats

ChrsMark · 2020-10-27T13:36:33Z

What does this PR do?

adds kubernetes.host.hostname on node metadata,
attach node metadata on pod metadata by default (without add_resource_metadata required to enabled) for both events coming from autodiscovered pods but also for events enriched by add_kubernetes_metadata
attach namespace metadata on pod metadata by default (without add_resource_metadata required to enabled) for both events coming from autodiscovered pods but also for events enriched by add_kubernetes_metadata
Enables always add_resource_metadata section for autodiscover and add_kubernetes_metadata.

Why is it important?

This information is useful in various ways. See more on the respective issue: #20434

How to test this PR manually

Use autodiscover config like:

Test with autodiscover provider

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 Redis module is enabled and that kubernetes.node.hostname field is populated.

Test with add_kubernetes_metadata processor

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

Make sure that kubernetes.node.hostname field is populated as well as kubernetes.node.* and kubernetes.namespace.*.

Related issues

Closes Add kubernetes.node.hostname field with the actual hostname of the node #20434

Screenshots

Filebeat & add_kubernetes_metadata:

Autodiscover:

elasticmachine · 2020-10-27T15:02:37Z

💚 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 #22189 updated]
Start Time: 2020-11-11T05:22:06.978+0000
Duration: 75 min 33 sec

Test stats 🧪

Test	Results
Failed	0
Passed	16413
Skipped	1344
Total	17757

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

elasticmachine · 2020-10-29T14:15:58Z

Pinging @elastic/integrations-platforms (Team:Platforms)

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

elasticmachine · 2020-10-29T14:25:38Z

Pinging @elastic/ingest-management (Team:Ingest Management)

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

elasticmachine · 2020-10-29T16:40:00Z

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	16413
Skipped	1344
Total	17757

…stname

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

…stname

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

ChrsMark · 2020-11-05T08:59:25Z

@jsoriano @exekias this is in a reviewable state right now so feel free to have a look and comment.

jsoriano · 2020-11-05T12:36:25Z

libbeat/processors/add_kubernetes_metadata/_meta/fields.yml

+        - name: node.hostname
+          type: keyword
+          description: >
+            Kubernetes hostname as reported by the node’s kernel


Thinking about this comment #20434 (comment), perhaps we should store this value directly in host.name (instead of storing it in kubernetes.node.hostname). What I am not sure is if then we should also populate host.id or other inventory fields and with what values.

Other option can be to postpone the decision and copy the field if we decide to use host.name.

@exekias @kaiyan-sheng wdyt?

Using host.name sounds like a good idea to me, this complies with the current definition we use. It would be good to do some testing to see if what add_host_metadata reports is the same thing reported by Kubernetes. For instance, when you change the hostname by hand, with and without setting a fqdn.

I think we need a deeper discussion around where host.id should be set, what will happen when add_host_metadata kicks in but host.name is already set by add_kubernetes_metadata? What will happen if add_host_metadata happens before add_kubernetes_metadata?

That sounds good to me! How about keeping the scope of this PR to just adding the node's metadata and opening a follow up issue so as to investigate further about host.name in separate story?

How about keeping the scope of this PR to just adding the node's metadata and opening a follow up issue so as to investigate further about host.name in separate story?

LGTM, worst case we will duplicate this field.

I think we need a deeper discussion around where host.id should be set, what will happen when add_host_metadata kicks in but host.name is already set by add_kubernetes_metadata? What will happen if add_host_metadata happens before add_kubernetes_metadata?

Agree. This is definitely worth some time to test and discuss. When users have more than one processor enabled for Metricbeat, overwriting the same fields will be a problem. We also have this issue to discuss moving adding host.name into add_host_metadata. But then both host.id and host.name will have this same issue.

jsoriano · 2020-11-05T12:44:59Z

libbeat/common/kubernetes/metadata/node.go


 	meta := n.resource.Generate("node", obj, opts...)
 	// TODO: Add extra fields in here if need be
+	meta.Put("node.hostname", n.hostname)


Shouldn't we use the hostname in obj.Status.Addresses[...].Address? I am concerned that when using cluster scope we can be setting here the hostname of the observer, and not the one the event belongs to.

++ good catch!

jsoriano · 2020-11-05T12:46:46Z

libbeat/common/kubernetes/metadata/metadata.go

+	nodeWatcher kubernetes.Watcher,
+	namespaceWatcher kubernetes.Watcher,
+	metaConf *AddResourceMetadataConfig,
+	hostname string) MetaGen {


We are passing the hostname in many places, I wonder if we may be using the wrong hostname when using cluster scope.

++ good catch!

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

ChrsMark · 2020-11-09T08:07:40Z

@jsoriano thanks for the suggestion! I made a change to this direction so feel free to have another look :)

@exekias @kaiyan-sheng any thoughts about #22189 (comment)?

ChrsMark · 2020-11-10T09:50:47Z

Looks good, only a comment about docs. As tnode and namespace metadata is always added, we should probably remove mentions to add_resource_metadata.*.enabled. Not sure if other options in add_resource_metadata are still used.

👍🏼 Thanks for the review @jsoriano! I ended up removing add_resource_metadata completely. The only concern about this could be the volume of labels and annotations that might be too big. With this change we IncludeLabels & IncludeAnnotations by default for node and namespace of a Pod. @exekias do you think this is something we can do?
Maybe this is a case of add_resource_metadata being useful and maybe we could keep it only for cases where we want to restrict the volume of labels/annotations that are added. I'm in two minds here 😅 so I would love your feedback.

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

jsoriano · 2020-11-10T10:07:08Z

With this change we IncludeLabels & IncludeAnnotations by default for node and namespace of a Pod.

I think we need to keep something to selectively include annotations, maybe we can keep add_resource_metadata only for this. Or, to have more simple options, what about using for that the same config as in the provider and the processor?

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

ChrsMark · 2020-11-10T10:56:45Z

++ Aggreed @jsoriano ! Tried to take it to this direction in my last commit (75cb108)

jsoriano

👍

CHANGELOG.next.asciidoc

Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>

…stname

(cherry picked from commit 95a88a6)

ChrsMark added the Team:Platforms Label for the Integrations - Platforms team label Oct 27, 2020

ChrsMark self-assigned this Oct 27, 2020

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 Oct 27, 2020

Add hostname info for Kubernetes node

f9a64cb

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

ChrsMark force-pushed the add_kubernetes_hostname branch from 930933e to f9a64cb Compare October 29, 2020 14:13

ChrsMark added [zube]: In Review review labels Oct 29, 2020

ChrsMark marked this pull request as ready for review October 29, 2020 14:15

add changelog

f2cb9de

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

botelastic bot added the Team:Ingest Management label Oct 29, 2020

make update

60cc121

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

ChrsMark requested a review from a team as a code owner October 29, 2020 15:32

ChrsMark added 2 commits November 2, 2020 10:52

Merge remote-tracking branch 'upstream/master' into add_kubernetes_ho…

18905ca

…stname

Add namespaceMeta and nodeMeta withing Pod's meta

390bb52

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

ChrsMark changed the title ~~Add hostname info for Kubernetes node~~ Kubernetes metadata enhancements Nov 4, 2020

ChrsMark added 6 commits November 4, 2020 12:47

Update changelog

5ff376f

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

Merge remote-tracking branch 'upstream/master' into add_kubernetes_ho…

aa30490

…stname

Add default config

36a72ae

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

Move defult config init to function

d378aa7

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

Fix tests

0e30e66

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

fmt

36da577

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

jsoriano reviewed Nov 5, 2020

View reviewed changes

Obtain hostname from resource status directly

7c1651c

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

Review changes

ef89838

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

Bring back add_resource_metadata section

75cb108

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

jsoriano approved these changes Nov 10, 2020

View reviewed changes

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved

Update CHANGELOG.next.asciidoc

8002c9c

Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>

jsoriano approved these changes Nov 10, 2020

View reviewed changes

This was referenced Nov 10, 2020

Unify PodMeta generation for autodiscover provider and add_kubernetes_metadata processor #22517

Closed

Consider copying kubernetes.node.hostname to host.name #22519

Closed

ChrsMark requested review from a team as code owners November 11, 2020 05:17

botelastic bot added the Team:Automation Label for the Observability productivity team label Nov 11, 2020

ChrsMark force-pushed the add_kubernetes_hostname branch from c80b612 to 8002c9c Compare November 11, 2020 05:20

ChrsMark removed the Team:Automation Label for the Observability productivity team label Nov 11, 2020

Merge remote-tracking branch 'upstream/master' into add_kubernetes_ho…

ea6f0e5

…stname

ChrsMark removed request for a team November 11, 2020 05:22

ChrsMark merged commit 95a88a6 into elastic:master Nov 11, 2020

ChrsMark mentioned this pull request Nov 11, 2020

Cherry-pick #22189 to 7.x: Kubernetes metadata enhancements #22536

Merged

ChrsMark added the v7.11.0 label Nov 11, 2020

ChrsMark added a commit to ChrsMark/beats that referenced this pull request Nov 11, 2020

Kubernetes metadata enhancements (elastic#22189)

45ae840

(cherry picked from commit 95a88a6)

ChrsMark added a commit that referenced this pull request Nov 12, 2020

Kubernetes metadata enhancements (#22189) (#22536)

e9df1af

(cherry picked from commit 95a88a6)

This was referenced Nov 20, 2020

Handle errors about k8s api nodes permissions #22683

Closed

Fix k8s watcher issue when node access to list nodes and ns #22714

Merged

ChrsMark added the test-plan Add this PR to be manual test plan label Dec 16, 2020

mantipin mentioned this pull request Mar 2, 2021

Kubernetes logs are not enriched with namespace annotations via add_resource_metadata #24288

Closed

This was referenced Nov 21, 2023

[Kubernetes] Remove extra base fields for state datastreams elastic/integrations#8393

Merged

[Metricbeat autodiscover][Provider Kubernetes] Add condition to node/namespace watchers #37181

Closed

Conversation

ChrsMark commented Oct 27, 2020 • 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 manually

Test with autodiscover provider

Test with add_kubernetes_metadata processor

Related issues

Screenshots

Uh oh!

elasticmachine commented Oct 27, 2020 • 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 🧪

Uh oh!

elasticmachine commented Oct 29, 2020

Uh oh!

elasticmachine commented Oct 29, 2020

Uh oh!

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

Uh oh!

💚 Flaky test report

Test stats 🧪

Uh oh!

ChrsMark commented Nov 5, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrsMark commented Nov 9, 2020

Uh oh!

ChrsMark commented Nov 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsoriano commented Nov 10, 2020

Uh oh!

ChrsMark commented Nov 10, 2020

Uh oh!

jsoriano left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ChrsMark commented Oct 27, 2020 •

edited

Loading

elasticmachine commented Oct 27, 2020 •

edited by jenkins-beats-ci bot

Loading

elasticmachine commented Oct 29, 2020 •

edited by jenkins-beats-ci bot

Loading

ChrsMark commented Nov 10, 2020 •

edited

Loading