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

Align k8s metadata configurations in Kubernetes module: add addResourceMetadata config #29133

Merged

Conversation

tetianakravchenko
Copy link
Contributor

@tetianakravchenko tetianakravchenko commented Nov 24, 2021

Signed-off-by: Tetiana Kravchenko tetiana.kravchenko@elastic.co

What does this PR do?

add configuration option for add_resource_metadata in kubernetes module.

Why is it important?

align configuration: add_resource_metadata is already available in add_kubernetes_metadata processor and the kubernetes provider

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.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

metricset.name: pod:

Screenshot 2021-11-25 at 15 51 10

metricset.name: container

Screenshot 2021-11-25 at 15 53 18

metricset.name: state_service
Screenshot 2021-11-25 at 15 54 25

Logs

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 24, 2021
@mergify
Copy link
Contributor

mergify bot commented Nov 24, 2021

This pull request does not have a backport label. Could you fix it @tetianakravchenko? 🙏
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.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 24, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 24, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-11-30T14:42:58.280+0000

  • Duration: 146 min 46 sec

  • Commit: 1ab0304

Test stats 🧪

Test Results
Failed 0
Passed 48062
Skipped 4157
Total 52219

💚 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!)

Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

I left some minors but overall is looking good already. We will also need to add the setting in documentation I guess.

Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

Looks good, we only need to ensure that we stop the watchers properly.

@tetianakravchenko tetianakravchenko marked this pull request as ready for review November 29, 2021 15:06
@tetianakravchenko tetianakravchenko changed the title use addResourceMetadata config instead of hardcoded bool Align k8s metadata configurations in Kubernetes module: add addResourceMetadata config Nov 29, 2021
@MichaelKatsoulis MichaelKatsoulis added the Team:Integrations Label for the Integrations team label Nov 30, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 30, 2021
@MichaelKatsoulis
Copy link
Contributor

@tetianakravchenko you should add a Changelog entry as well

@tetianakravchenko
Copy link
Contributor Author

/test

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
…er and nsWatcher to enricher

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
…nfig that leads to 'failed to parse field' error

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
…eMetadataWatchers; return resource watcher if nodeWatcher/nsWatcher failed to be created

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
@tetianakravchenko
Copy link
Contributor Author

@MichaelKatsoulis please have a look 1ab0304

Copy link
Contributor

@MichaelKatsoulis MichaelKatsoulis left a comment

Choose a reason for hiding this comment

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

LGTM! You can merge it!

@tetianakravchenko tetianakravchenko merged commit 95bdebf into elastic:master Dec 1, 2021
@tetianakravchenko tetianakravchenko deleted the k8s-metadata-config branch December 1, 2021 12:08
@ChrsMark
Copy link
Member

ChrsMark commented Dec 8, 2021

/package

@MichaelKatsoulis
Copy link
Contributor

@Mergifyio backport 7.17

@mergify
Copy link
Contributor

mergify bot commented Sep 21, 2022

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

@MichaelKatsoulis
Copy link
Contributor

@Mergifyio backport 7.17

@mergify
Copy link
Contributor

mergify bot commented Sep 21, 2022

backport 7.17

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Sep 21, 2022
…ceMetadata config (#29133)

* use addResourceMetadata config instead of hardcoded bool

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* revert config for local dev

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* update doc; use common function to create all watchers; add nodeWatcher and nsWatcher to enricher

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* revert ek_stack

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* add stop watchers; adjust test for dedoting labels; fix overriding config that leads to 'failed to parse field' error

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* adjust log messages; check that watcher is not nil

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* fix error message format; rename getPodMetadataWatchers -> getResourceMetadataWatchers; return resource watcher if nodeWatcher/nsWatcher failed to be created

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

* add changelog

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
(cherry picked from commit 95bdebf)

# Conflicts:
#	libbeat/autodiscover/providers/kubernetes/pod.go
#	libbeat/autodiscover/providers/kubernetes/pod_test.go
#	libbeat/common/kubernetes/metadata/metadata.go
#	libbeat/common/kubernetes/metadata/pod.go
#	libbeat/common/kubernetes/metadata/pod_test.go
#	libbeat/processors/add_kubernetes_metadata/indexers_test.go
#	metricbeat/docs/modules/kubernetes.asciidoc
#	metricbeat/metricbeat.reference.yml
#	metricbeat/module/kubernetes/_meta/config.reference.yml
#	metricbeat/module/kubernetes/util/kubernetes.go
#	x-pack/elastic-agent/pkg/composable/providers/kubernetes/pod.go
#	x-pack/metricbeat/metricbeat.reference.yml
MichaelKatsoulis added a commit to MichaelKatsoulis/beats that referenced this pull request Sep 22, 2022
MichaelKatsoulis added a commit that referenced this pull request Sep 29, 2022
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip Skip notification from the automated backport with mergify Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Align k8s metadata configurations in Kubernetes module
4 participants