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

Implement default process & runtime metrics #61

Closed
wants to merge 43 commits into from
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
8097604
Some of the linux process metrics
MrLotU Feb 15, 2020
69ba436
Some of these things are non Int32 on Linux
MrLotU Feb 15, 2020
ad29d87
Add logging
MrLotU Feb 15, 2020
24882ac
Add logging
MrLotU Feb 15, 2020
55970f5
Add logging
MrLotU Feb 15, 2020
e276789
Temp disable loop
MrLotU Feb 15, 2020
ce0ab69
Add logging
MrLotU Feb 15, 2020
928d094
Add logging
MrLotU Feb 15, 2020
dde317d
Add logging
MrLotU Feb 15, 2020
9a975d1
Add logging
MrLotU Feb 15, 2020
c6b8fba
cleanup
MrLotU Feb 15, 2020
3a8bb45
Address remarks
MrLotU Feb 21, 2020
027477f
Sanity script
MrLotU Feb 21, 2020
ffda628
Address most comments
MrLotU Mar 26, 2020
47cb74a
Merge branch 'master' into LotU-ProcessMetrics
MrLotU Mar 26, 2020
3f5de0e
Sanity
MrLotU Mar 26, 2020
286ab64
Move to Int
MrLotU Mar 26, 2020
1b3be71
Cleanup
MrLotU Mar 26, 2020
a072b5d
Remove Foundation dependency
MrLotU Apr 5, 2020
a044d14
Merge branch 'master' into LotU-ProcessMetrics
MrLotU Apr 5, 2020
00cc3b0
Address Tomerds comments
MrLotU May 22, 2020
0cf0d17
Merge branch 'master' into LotU-ProcessMetrics
MrLotU May 22, 2020
3f98a8f
Unsafe pointer updates
MrLotU May 22, 2020
78dcb03
Merge branch 'LotU-ProcessMetrics' of https://github.com/MrLotU/swift…
MrLotU May 22, 2020
9ab2612
Add #if check back in
MrLotU May 22, 2020
8ffec02
Review comments
MrLotU Jul 11, 2020
606163b
Review remarks
MrLotU Jul 16, 2020
71e992f
Rework to DispatchSourceTimer & fix tests
MrLotU Jul 23, 2020
85fd683
Remove magic numbers
MrLotU Jul 23, 2020
27bd191
Merge branch 'master' into LotU-ProcessMetrics
MrLotU Jul 23, 2020
5c82101
Sanity script
MrLotU Jul 23, 2020
fa82705
Merge branch 'LotU-ProcessMetrics' of https://github.com/MrLotU/swift…
MrLotU Jul 23, 2020
ead6f1d
Make Swift 5.0 work
MrLotU Jul 28, 2020
7193cef
Sanity
MrLotU Jul 28, 2020
a43cab2
Merge branch 'main' into LotU-ProcessMetrics
MrLotU Sep 18, 2020
3a6f1a1
Merge branch 'main' into LotU-ProcessMetrics
MrLotU Oct 9, 2020
9317fd1
Refactor SystemMetrics to a seperate module
MrLotU Nov 6, 2020
2da692d
Merge branch 'LotU-ProcessMetrics' of https://github.com/MrLotU/swift…
MrLotU Nov 6, 2020
cbb8a02
Merge branch 'main' into LotU-ProcessMetrics
MrLotU Nov 6, 2020
9c95321
Add back bootstrapWithSystemMetrics helper
MrLotU Nov 6, 2020
73baa5a
Merge branch 'LotU-ProcessMetrics' of https://github.com/MrLotU/swift…
MrLotU Nov 6, 2020
adacbf2
Cleanup
MrLotU Nov 6, 2020
8e9b22f
Refactor the way the locks are used
MrLotU Nov 9, 2020
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
31 changes: 31 additions & 0 deletions Sources/CoreMetrics/Metrics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
//
//===----------------------------------------------------------------------===//

import enum Dispatch.DispatchTimeInterval

// MARK: User API

extension Counter {
Expand Down Expand Up @@ -363,6 +365,8 @@ public enum MetricsSystem {
fileprivate static let lock = ReadWriteLock()
fileprivate static var _factory: MetricsFactory = NOOPMetricsHandler.instance
fileprivate static var initialized = false
fileprivate static var systemMetricsHandler: SystemMetricsHandler?
fileprivate static var systemMetricsInitialized = false
MrLotU marked this conversation as resolved.
Show resolved Hide resolved

/// `bootstrap` is an one-time configuration function which globally selects the desired metrics backend
/// implementation. `bootstrap` can be called at maximum once in any given program, calling it more than once will
Expand All @@ -378,6 +382,33 @@ public enum MetricsSystem {
}
}

/// `bootstrapSystemMetrics` is an one-time configuration function which globally enables system level metrics.
/// `bootstrapSystemMetrics` can be only called once, unless cancelled usung `cancelSystemMetrics`, calling it more
/// than once without cancelling will lead to undefined behaviour, most likely a crash.
///
/// - parameters:
/// - pollInterval: The interval at which system metrics should be updated.
/// - systemMetricsType: The type of system metrics to use. If none is provided this defaults to
/// `LinuxSystemMetrics` on Linux platforms and `NOOPSystemMetrics` on all other platforms.
/// - labels: The labels to use for generated system metrics.
public static func bootstrapSystemMetrics(pollInterval interval: DispatchTimeInterval = .seconds(2), systemMetricsType: SystemMetrics.Type? = nil, labels: SystemMetricsLabels) {
MrLotU marked this conversation as resolved.
Show resolved Hide resolved
self.lock.withWriterLockVoid {
precondition(!self.systemMetricsInitialized, "metrics system can only be initialized once per process. currently used factory: \(self._factory)")
self.systemMetricsHandler = SystemMetricsHandler(pollInterval: interval, systemMetricsType: systemMetricsType, labels: labels)
self.systemMetricsInitialized = true
}
}

/// Cancels the collection of system metrics. Calling this unlocks `bootstrapSystemMetrics` so it can be
/// called again.
public static func cancelSystemMetrics() {
self.lock.withWriterLockVoid {
self.systemMetricsHandler?.cancelSystemMetrics()
self.systemMetricsHandler = nil
self.systemMetricsInitialized = false
}
}

// for our testing we want to allow multiple bootstrapping
internal static func bootstrapInternal(_ factory: MetricsFactory) {
self.lock.withWriterLock {
Expand Down
198 changes: 198 additions & 0 deletions Sources/CoreMetrics/SystemMetrics.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift Metrics API open source project
//
// Copyright (c) 2018-2020 Apple Inc. and the Swift Metrics API project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of Swift Metrics API project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

import Foundation
MrLotU marked this conversation as resolved.
Show resolved Hide resolved

internal class SystemMetricsHandler {
fileprivate let queue = DispatchQueue(label: "com.apple.CoreMetrics.SystemMetricsHandler", qos: .background)
fileprivate let timeInterval: DispatchTimeInterval
fileprivate let systemMetricsType: SystemMetrics.Type
fileprivate let labels: SystemMetricsLabels
fileprivate let processId: Int
fileprivate var task: DispatchWorkItem?

init(pollInterval interval: DispatchTimeInterval = .seconds(2), systemMetricsType: SystemMetrics.Type? = nil, labels: SystemMetricsLabels) {
self.timeInterval = interval
if let systemMetricsType = systemMetricsType {
self.systemMetricsType = systemMetricsType
} else {
#if os(Linux)
self.systemMetricsType = LinuxSystemMetrics.self
#else
self.systemMetricsType = NOOPSystemMetrics.self
#endif
}
MrLotU marked this conversation as resolved.
Show resolved Hide resolved
self.labels = labels
self.processId = Int(ProcessInfo.processInfo.processIdentifier)

self.task = DispatchWorkItem(qos: .background, block: {
let metrics = self.systemMetricsType.init(pid: self.processId)
if let vmem = metrics.virtualMemoryBytes { Gauge(label: self.labels.label(for: \.virtualMemoryBytes)).record(vmem) }
if let rss = metrics.residentMemoryBytes { Gauge(label: self.labels.label(for: \.residentMemoryBytes)).record(rss) }
if let start = metrics.startTimeSeconds { Gauge(label: self.labels.label(for: \.startTimeSeconds)).record(start) }
if let cpuSeconds = metrics.cpuSeconds { Gauge(label: self.labels.label(for: \.cpuSecondsTotal)).record(cpuSeconds) }
if let maxFds = metrics.maxFds { Gauge(label: self.labels.label(for: \.maxFds)).record(maxFds) }
if let openFds = metrics.openFds { Gauge(label: self.labels.label(for: \.openFds)).record(openFds) }
})

self.updateSystemMetrics()
}

internal func cancelSystemMetrics() {
self.task?.cancel()
self.task = nil
}

internal func updateSystemMetrics() {
self.queue.asyncAfter(deadline: .now() + self.timeInterval) {
guard let task = self.task else { return }
task.perform()
self.updateSystemMetrics()
}
}
}

public struct SystemMetricsLabels {
let prefix: String
let virtualMemoryBytes: String
MrLotU marked this conversation as resolved.
Show resolved Hide resolved
let residentMemoryBytes: String
let startTimeSeconds: String
let cpuSecondsTotal: String
let maxFds: String
let openFds: String

public init(prefix: String, virtualMemoryBytes: String, residentMemoryBytes: String, startTimeSeconds: String, cpuSecondsTotal: String, maxFds: String, openFds: String) {
self.prefix = prefix
self.virtualMemoryBytes = virtualMemoryBytes
self.residentMemoryBytes = residentMemoryBytes
self.startTimeSeconds = startTimeSeconds
self.cpuSecondsTotal = cpuSecondsTotal
self.maxFds = maxFds
self.openFds = openFds
}

func label(for keyPath: KeyPath<SystemMetricsLabels, String>) -> String {
return self.prefix + self[keyPath: keyPath]
}
}

public protocol SystemMetrics {
init(pid: Int)

var virtualMemoryBytes: Int? { get }
var residentMemoryBytes: Int? { get }
var startTimeSeconds: Int? { get }
var cpuSeconds: Int? { get }
var maxFds: Int? { get }
var openFds: Int? { get }
}
Copy link
Member

@tomerd tomerd Mar 27, 2020

Choose a reason for hiding this comment

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

how about we separate the function and the data? ie make SystemMetrics a simple struct that just carried the data (model object if you will) and introduce a protocol SystemMetricsReader

protocol SystemMetricsReader {
  func read(pid: Int) -> Result<SystemMetrics, Error>
}

or we can even have that as a function:

typealias SystemMetricsReader = (Int) -> Result<SystemMetrics, Error>

this way we dont need all these optionals and can simplify the try/catch stuff below - imo if it fails than no data better than partial data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reasoning for the optionals here is to always provide as much data as possible to users. For example, on Linux these metrics are retrieved in 3 parts. 1 of those parts could fail, but the other metrics will still be exposed.

Curious what your reasoning is for no data better than partial data. (IMO it's not partial data, just less data)

Copy link
Member

Choose a reason for hiding this comment

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

@MrLotU I am interested to understand the failure scenarios. i.e if some of these files you are trying to read does not exist, things must be bad no?


#if os(Linux)
private struct LinuxSystemMetrics: SystemMetrics {
let virtualMemoryBytes: Int?
let residentMemoryBytes: Int?
let startTimeSeconds: Int?
let cpuSeconds: Int?
let maxFds: Int?
let openFds: Int?

private enum SystemMetricsError: Error {
case FileNotFound
}

init(pid: Int) {
let pid = "\(pid)"
let ticks = _SC_CLK_TCK
do {
guard let statString =
try String(contentsOfFile: "/proc/\(pid)/stat", encoding: .utf8)
.split(separator: ")")
MrLotU marked this conversation as resolved.
Show resolved Hide resolved
.last
else { throw SystemMetricsError.FileNotFound }
let stats = String(statString)
.split(separator: " ")
MrLotU marked this conversation as resolved.
Show resolved Hide resolved
.map(String.init)
self.virtualMemoryBytes = Int(stats[safe: 20])
if let rss = Int(stats[safe: 21]) {
self.residentMemoryBytes = rss * _SC_PAGESIZE
} else {
self.residentMemoryBytes = nil
}
if let startTimeTicks = Int(stats[safe: 19]) {
self.startTimeSeconds = startTimeTicks / ticks
} else {
self.startTimeSeconds = nil
}
if let utimeTicks = Int(stats[safe: 11]), let stimeTicks = Int(stats[safe: 12]) {
let utime = utimeTicks / ticks, stime = stimeTicks / ticks
self.cpuSeconds = utime + stime
} else {
self.cpuSeconds = nil
}
} catch {
self.virtualMemoryBytes = nil
self.residentMemoryBytes = nil
self.startTimeSeconds = nil
self.cpuSeconds = nil
}
do {
guard
let line = try String(contentsOfFile: "/proc/\(pid)/limits", encoding: .utf8)
.split(separator: "\n")
.first(where: { $0.starts(with: "Max open file") })
.map(String.init) else { throw SystemMetricsError.FileNotFound }
self.maxFds = Int(line.split(separator: " ").map(String.init)[safe: 3])
} catch {
self.maxFds = nil
}
do {
let fm = FileManager.default,
items = try fm.contentsOfDirectory(atPath: "/proc/\(pid)/fd")
self.openFds = items.count
} catch {
self.openFds = nil
}
}
}
#else
#warning("System Metrics are not implemented on non-Linux platforms yet.")
#endif

private struct NOOPSystemMetrics: SystemMetrics {
let virtualMemoryBytes: Int?
let residentMemoryBytes: Int?
let startTimeSeconds: Int?
let cpuSeconds: Int?
let maxFds: Int?
let openFds: Int?

init(pid: Int) {
self.virtualMemoryBytes = nil
self.residentMemoryBytes = nil
self.startTimeSeconds = nil
self.cpuSeconds = nil
self.maxFds = nil
self.openFds = nil
}
}

private extension Array where Element == String {
subscript(safe index: Int) -> String {
guard index >= 0, index < endIndex else {
return ""
}

return self[index]
}
}
MrLotU marked this conversation as resolved.
Show resolved Hide resolved