[system][process] Add metric_type metadata for object fields, set stack restriction to 8.9.0 version #7660

tetianakravchenko · 2023-09-05T06:41:35Z

What does this PR do?

It is a follow up of this PR comment - #6493 (review)
to add all missing metric_type.

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

…8.9.0 version Signed-off-by: Tetiana Kravchenko <[email protected]>

packages/system/data_stream/process/fields/fields.yml

elasticmachine · 2023-09-05T06:58:45Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-09-27T07:35:21.633+0000
Duration: 16 min 38 sec

Test stats 🧪

Test	Results
Failed	0
Passed	151
Skipped	0
Total	151

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

elasticmachine · 2023-09-05T06:58:55Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`3/3`)	💚
Files	100.0% (`4/4`)	💚 5.556
Classes	100.0% (`4/4`)	💚 5.556
Methods	63.415% (`52/82`)	👎 -22.539
Lines	99.863% (`2924/2928`)	👍 13.84
Conditionals	100.0% (`0/0`)	💚

efd6 · 2023-09-06T01:36:19Z

packages/system/changelog.yml

+- version: "1.39.0"
+  changes:
+    - description: Add metric_type metadata for object fields, set stack restriction to 8.9.0 version
+      type: bugfix


enhancement?

lalit-satapathy

LGTM

lalit-satapathy · 2023-09-13T10:30:57Z

packages/system/data_stream/process/fields/fields.yml

@@ -541,11 +542,10 @@
              description: >
                The number of times that the memory limit (kmem_tcp.limit.bytes) was reached.

-            - name: stats.*


@tetianakravchenko, Any reason we are removing the mapping in multiple places for stats.* and we are only keeping the ones with stats.*.bytes

How will be the following field be mapped: system.process.cgroup.memory.stats.page_faults

Can we check the mapping of all the stats group under system.process ?

@lalit-satapathy I am not sure why we initially had stats.* format.

all metrics that includes * in the name are included now in dynamic_templates:

How will be the following field be mapped: system.process.cgroup.memory.stats.page_faults

system.process.cgroup.memory.stats.page_faults is defined here -

integrations/packages/system/data_stream/process/fields/fields.yml

Lines 602 to 606 in 87e0e38

- name: stats.page_faults

type: long

metric_type: counter

description: >

Number of times that a process in the cgroup triggered a page fault.

mapping for this field:

Can we check the mapping of all the stats group under system.process ?

Stats object mapping under `system.process.cgroup.memory`

"stats": { "properties": { "active_anon": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "active_file": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "anon": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "anon_thp": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "cache": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "file": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "file_dirty": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "file_mapped": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "file_thp": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "file_writeback": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "hierarchical_memory_limit": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "hierarchical_memsw_limit": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "htp_collapse_alloc": { "type": "long" }, "inactive_anon": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "inactive_file": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "kernel_stack": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "major_page_faults": { "type": "long", "time_series_metric": "counter" }, "mapped_file": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "page_activate": { "type": "long" }, "page_deactivate": { "type": "long" }, "page_faults": { "type": "long", "time_series_metric": "counter" }, "page_lazy_free": { "type": "long" }, "page_lazy_freed": { "type": "long" }, "page_refill": { "type": "long" }, "page_scan": { "type": "long" }, "page_steal": { "type": "long" }, "page_tables": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "pages_in": { "type": "long", "time_series_metric": "counter" }, "pages_out": { "type": "long", "time_series_metric": "counter" }, "per_cpu": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "rss": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "rss_huge": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "shmem": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "shmem_thp": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "slab": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "slab_reclaimable": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "slab_unreclaimable": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "sock": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "swap": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "swap_cached": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "thp_fault_alloc": { "type": "long" }, "unevictable": { "properties": { "bytes": { "type": "long", "time_series_metric": "gauge" } } }, "workingset_activate_anon": { "type": "long" }, "workingset_activate_file": { "type": "long" }, "workingset_node_reclaim": { "type": "long" }, "workingset_refault_anon": { "type": "long" }, "workingset_refault_file": { "type": "long" }, "workingset_restore_anon": { "type": "long" }, "workingset_restore_file": { "type": "long" } } }

One thing I noticed that some fields are not defined in the fields folder and for this reason they are missing time_series_metric. I think it is fields that were added in this PR - elastic/beats#27242.

I think there should be another PR for adding missing fields, to not mix it up in this PR

Hey @fearful-symmetry I see that this field was introduced in this PR - #1539 by you, could you please check if I am missing here something? I don't really understand why this was initially introduced, in addition to the stats.*.bytes - https://github.com/elastic/integrations/pull/1539/files#diff-c594e593105a59a8530cec1e0670442dbe7b2cc59074900a65b96e6b4f95352fR481-R483

lalit-satapathy · 2023-09-26T09:32:19Z

all metrics that includes * in the name are included now in dynamic_templates:

It will be good, if we confirm that no fields are left un-mapped because we removed stats.*, stats.*.* mappings. Otherwise LGTM.

packages/system/data_stream/process/fields/fields.yml

Signed-off-by: Tetiana Kravchenko <[email protected]>

ritalwar

LGTM!

tetianakravchenko · 2023-09-27T10:20:53Z

@lalit-satapathy

It will be good, if we confirm that no fields are left un-mapped because we removed stats., stats..* mappings. Otherwise LGTM.

can you please explain how do you want me to confirm it?
I think it is not even possible to get unmapped fields since by default is used dynamic mapping

I've checked the latest system package version running in cloud (so without my changes):

so fields stats.* are mapped either as defined in fields file - example page_faults https://github.com/elastic/integrations/blob/main/packages/system/data_stream/process/fields/fields.yml#L602-L625 or defined by the dynamic mapping - note that some fields like page_activate are not defined in the fields file, but are exposed by the beats - https://github.com/elastic/beats/blob/main/metricbeat/module/system/process/_meta/data.json#L170

On my local setup with the version that include changes in this PR:

you can see that both mentioned fields are present as well - page_activate (not defined in list of fields) and page_faults (defined in list of fields)

lalit-satapathy

LGTM

elasticmachine · 2023-09-27T13:14:16Z

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

Add metric_type metadata for object fields, set stack restriction to …

ad3ac5d

…8.9.0 version Signed-off-by: Tetiana Kravchenko <[email protected]>

tetianakravchenko requested review from a team as code owners September 5, 2023 06:41

tetianakravchenko commented Sep 5, 2023

View reviewed changes

packages/system/data_stream/process/fields/fields.yml Show resolved Hide resolved

Update changelog.yml

a41ff55

tetianakravchenko requested a review from agithomas September 5, 2023 11:26

efd6 reviewed Sep 6, 2023

View reviewed changes

Change the type of changes to 'enhancement' in changelog

87e0e38

This was referenced Sep 6, 2023

[system][process] Enable tsdb for process datastream #7672

Merged

Enable TSDB for System integration #5848

Closed

lalit-satapathy reviewed Sep 12, 2023

View reviewed changes

lalit-satapathy reviewed Sep 13, 2023

View reviewed changes

tetianakravchenko requested a review from ritalwar September 26, 2023 09:23

ritalwar reviewed Sep 27, 2023

View reviewed changes

packages/system/data_stream/process/fields/fields.yml Outdated Show resolved Hide resolved

fix description of the stats.*.*.bytes field; fix merge conflicts

89a5aa6

Signed-off-by: Tetiana Kravchenko <[email protected]>

ritalwar approved these changes Sep 27, 2023

View reviewed changes

lalit-satapathy approved these changes Sep 27, 2023

View reviewed changes

tetianakravchenko merged commit ff961b5 into elastic:main Sep 27, 2023

andrewkroh added the Integration:system System label Jul 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[system][process] Add metric_type metadata for object fields, set stack restriction to 8.9.0 version #7660

[system][process] Add metric_type metadata for object fields, set stack restriction to 8.9.0 version #7660

tetianakravchenko commented Sep 5, 2023

elasticmachine commented Sep 5, 2023 •

edited

Loading

Build stats

Test stats 🧪

elasticmachine commented Sep 5, 2023 •

edited

Loading

efd6 Sep 6, 2023

lalit-satapathy left a comment

lalit-satapathy Sep 13, 2023

tetianakravchenko Sep 26, 2023 •

edited

Loading

tetianakravchenko Sep 26, 2023

lalit-satapathy commented Sep 26, 2023

ritalwar left a comment

tetianakravchenko commented Sep 27, 2023

lalit-satapathy left a comment

elasticmachine commented Sep 27, 2023

	- name: stats.page_faults
	type: long
	metric_type: counter
	description: >
	Number of times that a process in the cgroup triggered a page fault.

[system][process] Add metric_type metadata for object fields, set stack restriction to 8.9.0 version #7660

[system][process] Add metric_type metadata for object fields, set stack restriction to 8.9.0 version #7660

Conversation

tetianakravchenko commented Sep 5, 2023

What does this PR do?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Screenshots

elasticmachine commented Sep 5, 2023 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

elasticmachine commented Sep 5, 2023 • edited Loading

🌐 Coverage report

efd6 Sep 6, 2023

Choose a reason for hiding this comment

lalit-satapathy left a comment

Choose a reason for hiding this comment

lalit-satapathy Sep 13, 2023

Choose a reason for hiding this comment

tetianakravchenko Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

tetianakravchenko Sep 26, 2023

Choose a reason for hiding this comment

lalit-satapathy commented Sep 26, 2023

ritalwar left a comment

Choose a reason for hiding this comment

tetianakravchenko commented Sep 27, 2023

lalit-satapathy left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 27, 2023

elasticmachine commented Sep 5, 2023 •

edited

Loading

elasticmachine commented Sep 5, 2023 •

edited

Loading

tetianakravchenko Sep 26, 2023 •

edited

Loading