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

Introducing Deployment condition status metric in Kubernetes module #35999

Merged
merged 14 commits into from Jul 6, 2023

Conversation

gizas
Copy link
Contributor

@gizas gizas commented Jul 5, 2023

  • Enhancement

What does this PR do?

Introduces the kube_deployment_status_condition metric inside kubernetes.state_deployment metrciset. In more details a new field will be indexed called kubernetes.deployment.status

Why is it important?

Relates to elastic/elastic-agent#2995

Introduces

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

See information on elastic/elastic-agent#2995

Related issues

@gizas gizas requested a review from a team as a code owner July 5, 2023 10:05
@gizas gizas self-assigned this Jul 5, 2023
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 5, 2023
@mergify
Copy link
Contributor

mergify bot commented Jul 5, 2023

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @gizas? 🙏.
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

@gizas gizas added the Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team label Jul 5, 2023
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jul 5, 2023
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.

You will need to add the new field in the respective fields.yml file so as to define its mapping.

@@ -23,6 +23,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]


*Metricbeat*
- Introducing Deployment condition status metric in Kubernetes module {pull}35999[35999]
Copy link
Member

Choose a reason for hiding this comment

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

This is not a breaking change. Hence it should go in ==== Added section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jul 5, 2023

💚 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: 2023-07-06T10:52:11.996+0000

  • Duration: 81 min 22 sec

Test stats 🧪

Test Results
Failed 0
Passed 4985
Skipped 1039
Total 6024

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the 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
Contributor

@tetianakravchenko tetianakravchenko left a comment

Choose a reason for hiding this comment

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

@@ -59,6 +59,10 @@
"MetricSetFields": {
"name": "coredns",
"paused": false,
"status": {
"available": "true",
Copy link
Contributor

Choose a reason for hiding this comment

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

formatting seems to be not correct

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

/*
This is how deployment_status_condition field will be exported:

kube_deployment_status_condition{namespace="default",deployment="test-deployment",condition="Available",status="true"} 0
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be enough to add it in corresponding plain file - https://github.com/elastic/beats/blob/ac1d127d6868caff2f1bbbc670bbfff7f88930fc/metricbeat/module/kubernetes/_meta/test/ksm.v2.4.2.plain, instead of this comment

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 is already there see line 222

It is more alligned to this comment:

// These fields will be set to "true", "false", or "unknown" based on input that looks
// like this:
//
// kube_job_complete{namespace="default",job_name="timer-27074308",condition="true"} 1
// kube_job_complete{namespace="default",job_name="timer-27074308",condition="false"} 0
// kube_job_complete{namespace="default",job_name="timer-27074308",condition="unknown"} 0

I found it helpful when first looked the code, because you can not understand how the p.LabelMatric works from first glance.

Let me know if you think is obsolete and I can remove it

@gizas
Copy link
Contributor Author

gizas commented Jul 5, 2023

You will need to add the new field in the respective fields.yml file so as to define its mapping.

Done thanks !

"status": {
"available": "true",
"progressing": "true"
},
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Blew my brain :) Fixed thanks @ChrsMark

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
Co-authored-by: Chris Mark <chrismarkou92@gmail.com>
@gizas gizas requested a review from ChrsMark July 6, 2023 09:05
@gizas gizas merged commit c903cb0 into main Jul 6, 2023
27 checks passed
@gizas gizas deleted the deployment_condition branch July 6, 2023 12:37
Scholar-Li pushed a commit to Scholar-Li/beats that referenced this pull request Feb 5, 2024
…lastic#35999)

* adding deployment condition status

* Update CHANGELOG.next.asciidoc

* fixing unittests

---------

Co-authored-by: Chris Mark <chrismarkou92@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants