[System] Add missing metric_type metadata#6395
[System] Add missing metric_type metadata#6395tetianakravchenko merged 4 commits intoelastic:mainfrom
Conversation
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
🌐 Coverage report
|
| description: swap readahead pages | ||
| - name: readahead.cached | ||
| type: long | ||
| metric_type: counter |
There was a problem hiding this comment.
I am unable to find a useful resource to validate this.
However the aggregation function that matters most on this field is the rate() function, supported by the counter. So, i do not find any risk to mark the field as counter type.
@tetianakravchenko , have you got a chance to validate this such as the kernel documentation?
There was a problem hiding this comment.
no, I couldn't find as well, just found where is it set - https://github.com/elastic/beats/blob/main/metricbeat/module/linux/memory/data.go#L87-L88, and that it corresponds to swap_ra_hit, I assumed it should be similar to the swap_ra (swap.readahead.pages), that is already set as counter
Also when double-checking I noticed that it is actually defined in the linux module, not in the system module. And this field is not defined in the list of fields in metricbeat - https://github.com/elastic/beats/blob/main/metricbeat/module/system/memory/_meta/fields.yml and not defined in example in our documentation - https://www.elastic.co/guide/en/beats/metricbeat/8.8/metricbeat-metricset-system-memory.html
@elastic/elastic-agent-data-plane could you please double-check if this field was added correctly here, or should it be deleted, since it is not present in metricbeat?
There was a problem hiding this comment.
as discussed with @agithomas: I think we agreed that it should be safe to keep it as counter
discussion regarding if this fields should be removed or not is out of the scope of this PR and there should be created another issue for that. @elastic/elastic-agent-data-plane wdyt?
There was a problem hiding this comment.
yes, it is a counter. https://elixir.bootlin.com/linux/latest/source/mm/swap_state.c#L365
| description: swap readahead pages | ||
| - name: readahead.cached | ||
| type: long | ||
| metric_type: counter |
There was a problem hiding this comment.
yes, it is a counter. https://elixir.bootlin.com/linux/latest/source/mm/swap_state.c#L365
|
Package system - 1.31.1 containing this change is available at https://epr.elastic.co/search?package=system |
* add missing metric_type metadata Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> * fix the PR link in changelog Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co> --------- Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>
What does this PR do?
Cpu and Memory data_streams already contain the
metric_type, only those 2 fields were missing for some reason.In all system package are missing
metric_typefor those 2 fields and forprocess.cgroupmetrics - it will be a part for a different PR.Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
Related issues
Screenshots