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

[Kubernetest OTEL] Follow up enhancements #11310

Merged

Conversation

tetianakravchenko
Copy link
Contributor

Proposed commit message

Please explain:

  • WHAT:
    • change logo
    • fix package description
    • add consistency to the terminology: usage vs utilisation, remove pct
    • do not rely on generic.otel datastream name: changed to filter data_stream.dataset: *.otel, use metrics-*otel* for adhoc dataviews, for events use logs-*otel*
  • WHY: Address review comments of the first version of the kubernetes content package

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
@andrewkroh andrewkroh added dashboard Relates to a Kibana dashboard bug, enhancement, or modification. enhancement New feature or request Integration:kubernetes_otel Kubernetes OpenTelemetry Assets Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team [elastic/obs-cloudnative-monitoring] labels Oct 2, 2024
@tetianakravchenko
Copy link
Contributor Author

tetianakravchenko commented Oct 2, 2024

Hey @ChrsMark in this PR I think I've addressed your comments you've left in #10910 (review), mainly in the second commit - 071f1ef

Could we try to be consistent with the terminology, i.e. usage vs utilization vs pct? Our panel titles etc should be aligned with the semantic convention definitions. Hence we should only use usage or utilization where it applies (no need for pct, utilization is a ratio anyways).

  1. chnged name of those vizualisation to Usage
Screenshot 2024-10-02 at 19 24 03
  1. for Node overview vizualisations switched to utilization and changed formula for cpu to address [Draft] K8s otel overview dashboard #10910 (comment)

Could we consider leveraging the limits' utilization metrics as well? There was strong push-back by the community when we introduced the *.node.utilization metrics so relying only these might be risky in case the community decides to deprecate them. From my perspective these are nice to haves but we can rely to *.cpu.usage ones and the limit based utilizations.

to double-check: is this comment already updated? Now there is used both:
Screenshot 2024-10-02 at 19 27 58

Also in the panels it would be nice if we explicitly mention what utilization we display (ratio against the node's capacity VS ratio against the limits).

done

@gizas
Copy link
Contributor

gizas commented Oct 3, 2024

Just a reminder to update the id of the dashborad :kubernetes-opentelemetry-cluster-overview

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.

Thank's for addressing the comments!
LGTM

"params": {
"format": {
"id": "percent",
"params": {
"decimals": 2
}
},
"formula": "last_value(metrics.k8s.node.cpu.usage)/last_value(metrics.k8s.node.allocatable_cpu)",
Copy link
Member

Choose a reason for hiding this comment

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

FYI there is a plan to add back the k8s.node.cpu.utilization but it will take some time since it's happening through a deprecation process which will take a while. In that case the k8s.node.cpu.utilization will be a ratio against the Node's capacity not the Node's allocatable cpu (capacity>allocatable), but it can be decided later if it will be fine to switch to that metric directly or not.

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

@gizas

Just a reminder to update the id of the dashborad :kubernetes-opentelemetry-cluster-overview

Note: the name of the dashboard must follow this pattern: https://github.com/elastic/package-spec/blob/v3.2.2/spec/content/kibana/spec.yml#L12, so renamed to kubernetes_otel-cluster-overview
82c295a

@elasticmachine
Copy link

💚 Build Succeeded

History

@tetianakravchenko tetianakravchenko merged commit 3737c1e into elastic:main Oct 7, 2024
5 checks passed
@elastic-vault-github-plugin-prod

Package kubernetes_otel - 0.0.2 containing this change is available at https://epr.elastic.co/search?package=kubernetes_otel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dashboard Relates to a Kibana dashboard bug, enhancement, or modification. enhancement New feature or request Integration:kubernetes_otel Kubernetes OpenTelemetry Assets Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team [elastic/obs-cloudnative-monitoring]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants