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

expose systemd's MemoryCurrent value in metrics #87

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

evgeni
Copy link

@evgeni evgeni commented May 30, 2023

No description provided.

systemd/systemd.go Outdated Show resolved Hide resolved
Signed-off-by: Evgeni Golov <evgeni@golov.de>
@SuperQ
Copy link
Contributor

SuperQ commented Jun 6, 2023

Implementation seems fine. But doesn't this overlap with a number of different cAdvisor metrics? We've been trying to keep the systemd_exporter from also becoming a new cAdvisor.

@evgeni
Copy link
Author

evgeni commented Jun 6, 2023

Partially, I guess. You can certainly get the same numbers from cAdvisor (by looking at the cgroup systemd created), but given that systemd already has this data it felt natural to also export it here for setups that don't want/need all other features of cAdvisor?

(The old, broken memory metrics were removed in #68 and you said "Keep only metrics provided by systemd itself." :) )

@yangxiangyu
Copy link

need this feature.

@frittentheke
Copy link

Partially, I guess. You can certainly get the same numbers from cAdvisor (by looking at the cgroup systemd created), but given that systemd already has this data it felt natural to also export it here for setups that don't want/need all other features of cAdvisor?

(The old, broken memory metrics were removed in #68 and you said "Keep only metrics provided by systemd itself." :) )

@SuperQ I was just about to quote this exact commit when wondering about resource metrics for systemd units being gone in the recent master. I suppose it boils down to the question whether the systemd exporter should expose any resource metrics (and their limits) belonging to units?

If you invoke systemctl show some.service you see e.g.

[...]
CPUUsageNSec=34524810546022
MemoryCurrent=82944000
IOReadBytes=18446744073709551615
IOReadOperations=18446744073709551615
IOWriteBytes=18446744073709551615
IOWriteOperations=18446744073709551615
IPIngressBytes=[no data]
IPIngressPackets=[no data]
IPEgressBytes=[no data]
IPEgressPackets=[no data]
[...]

provided accounting is enabled for the particular resources via e.g.

[...]
CPUAccounting=yes
IOAccounting=no
BlockIOAccounting=yes
MemoryAccounting=no
TasksAccounting=yes
IPAccounting=no
[...]

Yes most if not all of those metrics will likely be available via cAdvisor since that exposes all of them cgroups. But it does so without any context of the system unit, apart from the slice name, right?
Having the systemd exporter to selectively export resource metrics for units which have them enabled, will greatly reduce cardinality and maybe also makes it easier to correlate with other unit metrics?

There even is a switch (https://github.com/google/cadvisor/blob/fbd519ba03978d54cb54ea7ed8ab9d6e3dd64590/docs/runtime_options.md?plain=1#L14) to disable exporting anything but "real" containers since cAdvisor is actually all about "containers".

@sturmf
Copy link

sturmf commented Dec 1, 2023

I also would vote for this PR 👏

I am running systemd-exporter with this patch and it works great and I now have a trifecta of exporters working hand in hand:

  • The node_exporter gives me the memory consumption overview of the whole system
  • The systemd_exporter gives me the memory consumption overview by systemd unit
  • And finally the process_exporter can be used to drill down to individual processes.

It really would be great if this can be merged into the codebase!

@shamil
Copy link

shamil commented Dec 11, 2023

Hey guys,

Just to understand the intention of this exporter. I came accross this exporter due it's capability to provide resource usage metrics (e.g. CPU and Memory) per systemd unit, apperantly this is not the case any longer. If we won't add support for CPU/Memory (such as this PR does), I'm wondering what's the scope of this exporter and how it is any better to what is exported by the node_exporter?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants