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

[stack monitoring][apm] use cgroup values for memory #96403

Closed
simitt opened this issue Apr 7, 2021 · 6 comments · Fixed by #97076
Closed

[stack monitoring][apm] use cgroup values for memory #96403

simitt opened this issue Apr 7, 2021 · 6 comments · Fixed by #97076

Comments

@simitt
Copy link
Contributor

simitt commented Apr 7, 2021

from #79050

Stack Monitoring should use these for calculating more accurate CPU and memory usage when running Beats and APM Server in containers.

While testing 7.13 on cloud I realized that the changes added in #90873 only covered using cgroup values for CPU, but not for memory usage.
Unfortunately I missed this during the PR review.

The collected cgroup values are stored in beats_stats.metrics.beat.cgroup.memory.*. I suggest we show the beats_stats.metrics.beat.cgroup.memory.mem.usage.bytes and GC Next (unchanged) for cgroup data.

@igoristic can you also add the memory changes for 7.13? At the moment it is a bit surprising to see CPU cgroup vs. memory system usage.

cc @elastic/apm-server @ruflin

@botelastic botelastic bot added the needs-team Issues missing a team label label Apr 7, 2021
@simitt simitt changed the title [stack monitoring][apm] cgroup values for memory [stack monitoring][apm] use cgroup values for memory Apr 7, 2021
@simitt simitt added the Team:Monitoring Stack Monitoring team label Apr 7, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring (Team:Monitoring)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Apr 7, 2021
@sgrodzicki sgrodzicki added this to the Stack Monitoring UI 7.13 milestone Apr 7, 2021
@igoristic igoristic self-assigned this Apr 12, 2021
@igoristic
Copy link
Contributor

@simitt
Do we need to add periods and quota in the calculation like we did for CPU?

@simitt
Copy link
Contributor Author

simitt commented Apr 14, 2021

There are no beats_stats.metrics.beat.cgroup.memory.stats.periods and beats_stats.metrics.beat.cgroup.memory.cfs.quota.us reported.

I suggest we

  • show beats_stats.metrics.beat.cgroup.memory.mem.usage.bytes
  • show beats_stats.metrics.beat.cgroup.memory.mem.limit.bytes (this will just be a straight line if a limit is set)
  • keep showing the GC Next information
  • remove the Allocated Memory and Process Total (as they are based on system metrics).

@axw are ok with this or did you have something different in mind?

@axw
Copy link
Member

axw commented Apr 14, 2021

Sounds good to me.

@ravikesarwani
Copy link
Contributor

remove the Allocated Memory and Process Total (as they are based on system metrics).

Can we please make sure the changes will work for both container and non container environments?
Also, it should work for APM and beats (filebeat, metricbeat ...) in general as well.

@simitt
Copy link
Contributor Author

simitt commented Apr 14, 2021

The changes should only affect container environments, and only affect the apm stack montioring.

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

Successfully merging a pull request may close this issue.

6 participants