-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Metricbeat] gcp: add GKE metricset #26824
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good start, keep pushing the missing items. You can copy the TODO list from the template: https://github.com/elastic/beats/issues/new?assignees=&labels=&template=module-checklist.md
I think that the next step is to make it working and passing lint.
metrics: | ||
- service: gke | ||
metric_types: | ||
- "container/cpu/core_usage_time" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To review available metrics, you can sync with @sorantis .
description: Memory limit of the container in bytes. Sampled every 60 seconds. | ||
- name: limit_utilization.value | ||
type: double | ||
description: The fraction of the memory limit that is currently in use on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we used pct
for percentages. See:
- name: memory.broker.pct
description: The percentage of the memory limit used.
type: scaled_float
format: percent
Sometimes you need an additional Javascript processor to convert to the right format:
processors:
- script:
lang: javascript
source: >
function process(event) {
var broker_memory_broker_pct = event.Get("activemq.broker.memory.broker.pct")
if (broker_memory_broker_pct != null) {
event.Put("activemq.broker.memory.broker.pct", broker_memory_broker_pct / 100.0)
}
var broker_memory_temp_pct = event.Get("activemq.broker.memory.temp.pct")
if (broker_memory_temp_pct != null) {
event.Put("activemq.broker.memory.temp.pct", broker_memory_temp_pct / 100.0)
}
var broker_memory_store_pct = event.Get("activemq.broker.memory.store.pct")
if (broker_memory_store_pct != null) {
event.Put("activemq.broker.memory.store.pct", broker_memory_store_pct / 100.0)
}
}
(based on the ActiveMQ module.
- name: used_bytes.value | ||
type: long | ||
description: Memory usage in bytes. Sampled every 60 seconds. | ||
- name: '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm.. empty name?
- name: memory | ||
type: group | ||
fields: | ||
- name: allocatable_bytes.value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to follow the Beats naming convention: https://www.elastic.co/guide/en/beats/devguide/current/event-conventions.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be max_bytes
or allocatable.bytes
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case - memory.allocable.bytes
. But this comment applies to all fields you submitted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:) Will update all occurrences
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this PR is still in draft, just want to bring elastic/ecs#1441 to your attention. As a part of the inventory schema work, we are planning to add some additional container metrics. It would be good if you can adopt these fields in this new metricset 🙂
ea66269
to
8d22db0
Compare
3af2e22
to
28f4a83
Compare
To keep consistency with the GCP documentation and implementation, I'll rename |
2a1afe1
to
7b36f30
Compare
4bc794d
to
2b93129
Compare
e73f364
to
7e70b34
Compare
#27416 has been merged Rebasing onto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Feel free to ship the dashboard separately.
When shipping a dashboard, please also add a screenshot into this folder under Metricbeat. Also reference the dashboard screenshot in the documentation. |
7e70b34
to
35abd50
Compare
I removed the dashboard... Now waiting on the CI |
Add a lightweight module to pull GCP GKE metrics. Metrics are all GA metrics as of today available for Kubernetes on GCP. Docs: https://cloud.google.com/monitoring/api/metrics_kubernetes
In elastic#27231 I discovered that all `gcp` metricsets do not respect the Beats naming conventions. In order to preserve consistency across metricset this commit reverts the changes that aligned the newly added metrics to naming conventions.
35abd50
to
9dbbbaa
Compare
Add a lightweight module to pull GCP GKE metrics. Metrics are all GA metrics as of today available for Kubernetes on GCP. Docs: cloud.google.com/monitoring/api/metrics_kubernetes (cherry picked from commit 1140740)
Add a lightweight module to pull GCP GKE metrics. Metrics are all GA metrics as of today available for Kubernetes on GCP. Docs: cloud.google.com/monitoring/api/metrics_kubernetes (cherry picked from commit 1140740)
Add a lightweight module to pull GCP GKE metrics. Metrics are all GA metrics as of today available for Kubernetes on GCP. Docs: cloud.google.com/monitoring/api/metrics_kubernetes (cherry picked from commit 1140740) Co-authored-by: endorama <526307+endorama@users.noreply.github.com>
Co-authored-by: Edoardo Tenani <edoardo.tenani@elastic.co>
Add a lightweight module to pull GCP GKE metrics. Metrics are all GA metrics as of today available for Kubernetes on GCP. Docs: cloud.google.com/monitoring/api/metrics_kubernetes
What does this PR do?
Add a lightweight module to pull GCP GKE metrics.
Metrics gathered are all GA metrics as of today available for Kubernetes on GCP.
GCP Docs: https://cloud.google.com/monitoring/api/metrics_kubernetes
Why is it important?
Implement GKE integration as requested in elastic/integrations#816
Dates
Tue, Aug 17 2021
Wed, Aug 18 2021
Tue, Sep 21 2021
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
You can build and run the
metricbeat
binary from this PR to collect GKE metrics. You may use this configuration forgcp.yml
module:Related issues
NOTE: due to this issue the fields do not align with Beats naming convention or ECS, to keep the behaviour across Metricsets consistent. Once this is addressed I'll tackle migrating all Metricsets to the correct format.
Use cases
Screenshots
Logs
Metricbeat Module / Dataset release checklist
This checklist is intended for Devs which create or update a module to make sure modules are consistent.
Modules
For a metricset to go GA, the following criterias should be met:gcp
module is currently inbeta
.Metricbeat module
data.json
exists and an automated way to generate it exists (go test -data
)