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

Issue #85: Added new metrics system.uptime #86

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
26 changes: 26 additions & 0 deletions src/main/connector/system/Linux/Linux.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,32 @@ monitors:
system.filesystem.usage{system.filesystem.state=free}: $5
system.filesystem.utilization{system.filesystem.state=used}: $6
system.filesystem.utilization{system.filesystem.state=free}: $7
system:
simple:
sources:
# Distribution;Version;Kernel
osInformation:
type: commandLine
commandLine: |
os_info=$(cat /etc/os-release | grep -E '^(NAME|VERSION="[0-9].*")' | tr -d '"')
Copy link
Member

Choose a reason for hiding this comment

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

It's actually more efficient and robust to "execute" /etc/os-release and then echo the required env variables. The below script will achieve the same result with much less processes:

. /etc/os-release
echo "$NAME;$VERSION;`uname -r`"

kernel_version=$(cat /proc/version | awk '{print $3}')
echo "$os_info; $kernel_version"
computes:
- type: awk
Copy link
Member

Choose a reason for hiding this comment

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

This compute step is unnecessary.

script: BEGIN { FS = "=" } { printf $2 ";"}
# Uptime (timestamp)
uptime:
type: commandLine
commandLine: expr $(date +%s) - $(awk '{print int($1)}' /proc/uptime)
Copy link
Member

Choose a reason for hiding this comment

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

Uptime is supposed to be the number of seconds since last boot, which is what we have in /proc/uptime (logically). You don't need to do all this logic, and you don't even need this uptime source.

Let's merge everything in 1 script:

      sources:
        osInformation:
          type: commandLine
          commandLine: |
            . /etc/os-release
            echo "$NAME;$VERSION;`uname -r`;`cut -d. -f1 /proc/uptime`"
      mapping:
        source: ${source::osInformation}
        attributes:
          id: $3
          name: $1
          version: $2
          os_version: $3
        metrics:
          system.uptime: $4

mapping:
source: ${source::osInformation}
attributes:
id: $3
name: $1
version: $2
os_version: $3
metrics:
system.uptime: ${source::uptime}
Copy link
Member

Choose a reason for hiding this comment

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

Urgl! @NassimBtk Is this even supported? 😅

Copy link
Member

Choose a reason for hiding this comment

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

Yes @bertysentry, I totally forgot we support that 🤦‍♂️, it will avoid unnecessary joins.

translations:
serviceLoadedTranslationTable:
not-found: "0"
Expand Down
6 changes: 5 additions & 1 deletion src/main/connector/system/System/System.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,8 @@ metrics:
system.linux.memory.available:
description: An estimate of how much memory is available for starting new applications, without causing swapping
type: UpDownCounter
unit: By
unit: By
system.uptime:
description: Timestamp of the system's last startup.
Copy link
Member

Choose a reason for hiding this comment

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

No, uptime is the time elapsed since last boot.

type: Gauge
unit: s
23 changes: 23 additions & 0 deletions src/main/connector/system/Windows/Windows.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -274,3 +274,26 @@ monitors:
system.disk.io_time: $6
system.disk.operation_time{disk.io.direction="read"}: $7
system.disk.operation_time{disk.io.direction="write"}: $8
system:
simple:
sources:
# SerialNumber;Caption;Version;BuildNumber
osInformation:
type: wmi
namespace: root\CIMv2
query: SELECT SerialNumber, Caption, Version, BuildNumber FROM Win32_OperatingSystem
# Uptime (timestamp)
uptime:
type: wmi
namespace: root\CIMv2
query: SELECT LastBootUpTime FROM Win32_OperatingSystem
Copy link
Member

Choose a reason for hiding this comment

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

Use SELECT SystemUpTime FROM Win32_PerfFormattedData_PerfOS_System to get the system uptime (and not the date of the last reboot).

mapping:
source: ${source::osInformation}
Copy link
Member

Choose a reason for hiding this comment

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

TBH, I think it would be safer to make a table join on osInformation and uptime, because mapping is supposed to you 1 resulting source. Using system.uptime: ${source::uptime} works just by chance 😅

attributes:
id: $1
serial_number: $1
name: $2
version: $4
os_version: $3
metrics:
system.uptime: ${source::uptime}