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

Jump to bottom

Core metrics collection - collect more memory related metrics #172146

Merged
merged 11 commits into from Nov 30, 2023
Merged

Core metrics collection - collect more memory related metrics #172146

merged 11 commits into from Nov 30, 2023

Conversation

pgayvallet
Copy link
Contributor

@pgayvallet pgayvallet commented Nov 29, 2023 •
edited by kibanamachine

Summary

Part of #171060

Add additional memory metrics for collection:

process:

  • process.memory.array_buffers_in_bytes the process.memoryUsage().arrayBuffers value
  • process.memory.external_in_bytes the process.memoryUsage().external value

cgroup: (v2 only)

  • os.cgroupMemory.current_in_bytes value from /sys/fs/cgroup/{group}/memory.current
  • os.cgroupMemory.swap_current_in_bytes value from /sys/fs/cgroup/{group}/memory.swap.current

@pgayvallet pgayvallet added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes Feature:Stack Monitoring v8.12.0 labels Nov 29, 2023
@pgayvallet pgayvallet mentioned this pull request Nov 29, 2023
Merged
5 tasks
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Investigations - Security Solution Cypress Tests #3 / Timelines Creates a timeline by clicking untitled timeline from bottom bar "before each" hook for "has a lock icon" "before each" hook for "has a lock icon"

Metrics [docs]

Unknown metric groups

API count

id before after diff
@kbn/core-metrics-server 55 56 +1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@pgayvallet pgayvallet marked this pull request as ready for review November 29, 2023 20:22
@pgayvallet pgayvallet requested a review from a team as a code owner November 29, 2023 20:22
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

pgayvallet
pgayvallet commented Nov 29, 2023
View reviewed changes
packages/core/metrics/core-metrics-collectors-server-internal/src/cgroup/cgroup.ts
Comment on lines +46 to +51
return await (this.isCgroup2
? gatherV2CgroupMetrics(this.cpuAcctPath!)
: gatherV1CgroupMetrics({
cpuAcctPath: this.cpuAcctPath!,
cpuPath: this.cpuPath!,
}));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored the signatures about v1/v2 detection a bit to get closer to reality (v2 -> a single group path)

packages/core/metrics/core-metrics-collectors-server-internal/src/cgroup/v2.ts
Comment on lines +54 to +66
async function readMemoryCurrent(group: string): Promise<number> {
const rawMemoryCurrent = (await fs.readFile(getCGroupFilePath(group, MEMORY_CURRENT_FILE)))
.toString()
.trim();
return parseInt(rawMemoryCurrent, 10);
}

async function readSwapCurrent(group: string): Promise<number> {
const rawMemoryCurrent = (await fs.readFile(getCGroupFilePath(group, MEMORY_SWAP_CURRENT_FILE)))
.toString()
.trim();
return parseInt(rawMemoryCurrent, 10);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both are "single constant" numeric files

TinaHeiligers
TinaHeiligers approved these changes Nov 29, 2023
View reviewed changes
Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

LGTM with a comment that the new metrics aren't logged in pattern mode.

packages/core/metrics/core-metrics-server-internal/src/logging/get_ops_metrics_log.ts
},
residentSetSizeInBytes: process?.memory?.resident_set_size_in_bytes,
Copy link
Contributor

Choose a reason for hiding this comment

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

If we want these to log to stdout in pattern format, we'll need to need to fiddle with processMemoryUsedInBytesMsg on line 21.

const processMemoryUsedInBytesMsg = processMemoryUsedInBytes
    ? `memory: ${numeral(processMemoryUsedInBytes).format('0.0b')} `
    : '';

Copy link
Contributor

Choose a reason for hiding this comment

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

The logs are fine in `json` format: 
{
"event": {
  "kind": "metric",
  "category": [
    "process",
    "host"
  ],
  "type": [
    "info"
  ]
},
"process": {
  "uptime": 72.65337025,
  "memory": {
    "heap": {
      "usedInBytes": 602026648,
      "totalInBytes": 695664640,
      "sizeLimit": 4345298944
    },
    "residentSetSizeInBytes": 1079312384,
    "externalInBytes": 7233984, <--- this PR
    "arrayBuffersInBytes": <--- this PR
  },
  "eventLoopDelay": 10.336272995850623,
  "eventLoopDelayHistogram": {
    "50": 10.346495,
    "95": 11.010047,
    "99": 12.140543
  },
  "eventLoopUtilization": {
    "active": 279.28487307039177,
    "idle": 4719.119918999997,
    "utilization": 0.05587480099920223
  },
  "pid": 63971
},
"host": {
  "os": {
    "load": {
      "1m": 6.076171875,
      "5m": 6.0078125,
      "15m": 6.32568359375
    }
  }
},
"service": {
  "node": {
    "roles": [
      "background_tasks",
      "ui"
    ]
  }
},
"ecs": {
  "version": "8.6.1"
},
"@timestamp": "2023-11-29T16:15:47.457-07:00",
"message": "memory: 574.1MB uptime: 0:01:12 load: [6.08,6.01,6.33] mean delay: 10.336 delay histogram: { 50: 10.346; 95: 11.010; 99: 12.141 } utilization: 0.05587",
"log": {
  "level": "DEBUG",
  "logger": "metrics.ops"
},
"trace": {
  "id": "095027dd7070903121c4316455216d32"
},
"transaction": {
  "id": "56fde480923f64a4"
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think it's fine. It's mostly for debug scenarios anyway, and we're using the json layout where it matters. But thanks for pointing that out!

@pgayvallet pgayvallet added the backport:prev-minor Backport to the previous minor version (i.e. one version back from main) label Nov 30, 2023
@pgayvallet pgayvallet merged commit b323fc9 into elastic:main Nov 30, 2023
35 checks passed
@kibanamachine kibanamachine mentioned this pull request Nov 30, 2023
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 30, 2023
@pgayvallet
Core metrics collection - collect more memory related metrics (elasti…
8feae01 
…c#172146)

## Summary

Part of elastic#171060

Add additional memory metrics for collection:

**process**:
- `process.memory.array_buffers_in_bytes` the
`process.memoryUsage().arrayBuffers` value
- `process.memory.external_in_bytes` the
`process.memoryUsage().external` value

**cgroup**:  (v2 only)
- `os.cgroupMemory.current_in_bytes` value from
`/sys/fs/cgroup/{group}/memory.current`
- `os.cgroupMemory.swap_current_in_bytes` value from
`/sys/fs/cgroup/{group}/memory.swap.current`

(cherry picked from commit b323fc9)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.11

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Nov 30, 2023
@kibanamachine @pgayvallet
[8.11] Core metrics collection - collect more memory related metrics (#…
4131708 
…172146) (#172271)

# Backport

This will backport the following commits from `main` to `8.11`:
- [Core metrics collection - collect more memory related metrics
(#172146)](#172146)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Pierre
Gayvallet","email":"pierre.gayvallet@elastic.co"},"sourceCommit":{"committedDate":"2023-11-30T10:47:46Z","message":"Core
metrics collection - collect more memory related metrics (#172146)\n\n##
Summary\r\n\r\nPart of
#171060 additional
memory metrics for collection:\r\n\r\n**process**:\r\n-
`process.memory.array_buffers_in_bytes`
the\r\n`process.memoryUsage().arrayBuffers` value\r\n-
`process.memory.external_in_bytes`
the\r\n`process.memoryUsage().external` value\r\n\r\n**cgroup**: (v2
only)\r\n- `os.cgroupMemory.current_in_bytes` value
from\r\n`/sys/fs/cgroup/{group}/memory.current`\r\n-
`os.cgroupMemory.swap_current_in_bytes` value
from\r\n`/sys/fs/cgroup/{group}/memory.swap.current`","sha":"b323fc90a81dd404686c1858a3c525ae1cc922ae","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Core","release_note:skip","Feature:Stack
Monitoring","backport:prev-minor","v8.12.0"],"number":172146,"url":"#172146
metrics collection - collect more memory related metrics (#172146)\n\n##
Summary\r\n\r\nPart of
#171060 additional
memory metrics for collection:\r\n\r\n**process**:\r\n-
`process.memory.array_buffers_in_bytes`
the\r\n`process.memoryUsage().arrayBuffers` value\r\n-
`process.memory.external_in_bytes`
the\r\n`process.memoryUsage().external` value\r\n\r\n**cgroup**: (v2
only)\r\n- `os.cgroupMemory.current_in_bytes` value
from\r\n`/sys/fs/cgroup/{group}/memory.current`\r\n-
`os.cgroupMemory.swap_current_in_bytes` value
from\r\n`/sys/fs/cgroup/{group}/memory.swap.current`","sha":"b323fc90a81dd404686c1858a3c525ae1cc922ae"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"#172146
metrics collection - collect more memory related metrics (#172146)\n\n##
Summary\r\n\r\nPart of
#171060 additional
memory metrics for collection:\r\n\r\n**process**:\r\n-
`process.memory.array_buffers_in_bytes`
the\r\n`process.memoryUsage().arrayBuffers` value\r\n-
`process.memory.external_in_bytes`
the\r\n`process.memoryUsage().external` value\r\n\r\n**cgroup**: (v2
only)\r\n- `os.cgroupMemory.current_in_bytes` value
from\r\n`/sys/fs/cgroup/{group}/memory.current`\r\n-
`os.cgroupMemory.swap_current_in_bytes` value
from\r\n`/sys/fs/cgroup/{group}/memory.swap.current`","sha":"b323fc90a81dd404686c1858a3c525ae1cc922ae"}}]}]
BACKPORT-->

Co-authored-by: Pierre Gayvallet <pierre.gayvallet@elastic.co>
@pgayvallet pgayvallet mentioned this pull request Nov 30, 2023
Merged
@pgayvallet pgayvallet mentioned this pull request Nov 30, 2023
Merged
@rudolf rudolf mentioned this pull request Nov 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TinaHeiligers TinaHeiligers TinaHeiligers approved these changes

Assignees
No one assigned
Labels
backport:prev-minor Backport to the previous minor version (i.e. one version back from main) Feature:Stack Monitoring release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.11.2 v8.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@pgayvallet @kibana-ci @elasticmachine @kibanamachine @TinaHeiligers