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.

Sign up for GitHub

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

feat(inputs.procstat): Add child level tag #16105

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
+77 −20
Open

feat(inputs.procstat): Add child level tag #16105

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
6c3c47d
feat(inputs.procstat): Add child level tag
tguenneguez Oct 29, 2024
bb65f20
Add level information as field and add avaibility to add as tag
tguenneguez Dec 5, 2024
6e4220d
Merge branch 'influxdata:master' into AddChildInfo
tguenneguez Dec 5, 2024
20b36fc
Update sample.conf doc
tguenneguez Dec 9, 2024
94a780a
Merge branch 'AddChildInfo' of https://github.com/tguenneguez/telegra…
tguenneguez Dec 9, 2024
f89948c
Merge branch 'influxdata:master' into AddChildInfo
tguenneguez Dec 9, 2024
5fc2efd
Corrige syntaxe
tguenneguez Dec 9, 2024
ad4f627
Merge branch 'AddChildInfo' of https://github.com/tguenneguez/telegra…
tguenneguez Dec 9, 2024
ebffcd7
Add tag level for procstat_lookup
tguenneguez Dec 10, 2024
d9f3e53
Merge branch 'influxdata:master' into AddChildInfo
tguenneguez Dec 10, 2024
276dd19
Corrige syntaxe
tguenneguez Dec 10, 2024
3073e64
Remove arguments not used
tguenneguez Dec 11, 2024
b8e6ea5
Corrige le bug de label
tguenneguez Dec 11, 2024
cbf1c24
Merge branch 'influxdata:master' into AddChildInfo
tguenneguez Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions plugins/inputs/procstat/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ Below are an example set of tags and fields:
- cgroup_full (when cgroup or systemd_unit is used with glob)
- supervisor_unit (when defined)
- win_service (when defined)
- parent_pid for child processes
- child_level for child processes
- fields:
- child_major_faults (int)
- child_minor_faults (int)
Expand Down
1 change: 1 addition & 0 deletions plugins/inputs/procstat/filter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ func (f *Filter) ApplyFilter() ([]processGroup, error) {
tags[k] = v
}
tags["parent_pid"] = strconv.FormatInt(int64(p.Pid), 10)
Copy link
Member

Choose a reason for hiding this comment

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

Why are you removing this here?

tags["child_level"] = strconv.FormatInt(int64(depth), 10)
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be a configuration option since it is a new tag

Copy link
Member

Choose a reason for hiding this comment

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

I agree as it will also likely produce high cardinality. How about adding this as a field instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the cardinality is higher on the parent_pid field so this change is less impactful than one might think.

Copy link
Member

Choose a reason for hiding this comment

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

In any case, we cannot add a new tag unconditionally...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I understand your point of view.
What option would you suggest to add this tag?
Is there a naming rule?
It also means that for each plugin each time new tags are added, an option is needed. Is this likely to create plugins with long configurations?
We could put:
tag_child_level.
If not defined, the tag is not added.
If tag_child_level=toto then the level is added in the tag "toto"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

up ? ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this should be a configuration option since it is a new tag
@DStrand1 Have you a name recommanded for activate new tag ?

Copy link
Member

Choose a reason for hiding this comment

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

[...] It also means that for each plugin each time new tags are added, an option is needed. Is this likely to create plugins with long configurations? [...]

That's why we prefer "lists" of what to add instead of boolean options. This way we have one option with many potential values. For this plugin there actually already is the tag_with option, so why don't you just add the child-level there?
Check the code for how this option is used to conditionally add tags...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.
I tryed, but I have 2 errors that I doesn't understand ...

children = append(children, processGroup{
processes: c,
tags: tags,
Expand Down
2 changes: 1 addition & 1 deletion plugins/inputs/procstat/procstat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ func (p *Procstat) findSupervisorUnits() ([]PidsTags, error) {
if err != nil {
return nil, fmt.Errorf("getting children for %d failed: %w", pid, err)
}
tags := map[string]string{"pattern": p.Pattern, "parent_pid": p.Pattern}
tags := map[string]string{"pattern": p.Pattern, "parent_pid": p.Pattern, "child_level": p.Pattern}
Copy link
Member

Choose a reason for hiding this comment

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

You need to conditionally add the child_level here!


// Handle situations where the PID does not exist
if len(pids) == 0 {
Expand Down
Loading