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

client: Return empty values when host stats fail #6349

Merged
merged 3 commits into from
Nov 20, 2019
Merged

client: Return empty values when host stats fail #6349

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b183e5
client: Return empty values when host stats fail
endocrimes Sep 18, 2019
c8ba938
client_stats: Always emit client stats
endocrimes Sep 18, 2019
4ba87cc
command: Improve metrics fail logging
endocrimes Sep 19, 2019
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
4 changes: 4 additions & 0 deletions client/allocrunner/taskrunner/task_runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1335,10 +1335,14 @@ func (tr *TaskRunner) emitStats(ru *cstructs.TaskResourceUsage) {

if ru.ResourceUsage.MemoryStats != nil {
tr.setGaugeForMemory(ru)
} else {
tr.logger.Debug("Skipping memory stats for allocation", "reason", "MemoryStats is nil")
Copy link
Contributor

Choose a reason for hiding this comment

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

I would adjust the casing, we typically have lower case message. Also, MemoryStats is nil is low level implementation; would be nice to have a user facing message, maybe something like:

Suggested change
tr.logger.Debug("Skipping memory stats for allocation", "reason", "MemoryStats is nil")
tr.logger.Debug("memory stats for alloc is unpopulated; skipping publishing them")

}

if ru.ResourceUsage.CpuStats != nil {
tr.setGaugeForCPU(ru)
} else {
tr.logger.Debug("Skipping cpu stats for allocation", "reason", "CpuStats is nil")
}
}

Expand Down
11 changes: 5 additions & 6 deletions client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2579,12 +2579,11 @@ func (c *Client) emitStats() {
next.Reset(c.config.StatsCollectionInterval)
if err != nil {
c.logger.Warn("error fetching host resource usage stats", "error", err)
continue
}

// Publish Node metrics if operator has opted in
if c.config.PublishNodeMetrics {
c.emitHostStats()
} else {
// Publish Node metrics if operator has opted in
if c.config.PublishNodeMetrics {
c.emitHostStats()
}
}

c.emitClientMetrics()
Expand Down
21 changes: 13 additions & 8 deletions client/stats/host.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package stats

import (
"fmt"
"math"
"runtime"
"sync"
Expand Down Expand Up @@ -117,39 +116,45 @@ func (h *HostStatsCollector) collectLocked() error {
// Determine up-time
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this and Collect() still return an error? May make sense to make them void functions instead? Simplifies client.go implementation.

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'm planning on going back and refactoring this a little bit next week. Mostly wanted to get something together to help folks with debugging.

uptime, err := host.Uptime()
if err != nil {
return err
h.logger.Error("failed to collect upstime stats", "error", err)
uptime = 0
}
hs.Uptime = uptime

// Collect memory stats
mstats, err := h.collectMemoryStats()
if err != nil {
return err
h.logger.Error("failed to collect memory stats", "error", err)
mstats = &MemoryStats{}
}
hs.Memory = mstats

// Collect cpu stats
cpus, ticks, err := h.collectCPUStats()
if err != nil {
return err
h.logger.Error("failed to collect cpu stats", "error", err)
cpus = []*CPUStats{}
ticks = 0
}
hs.CPU = cpus
hs.CPUTicksConsumed = ticks

// Collect disk stats
diskStats, err := h.collectDiskStats()
if err != nil {
return err
h.logger.Error("failed to collect disk stats", "error", err)
hs.DiskStats = []*DiskStats{}
}
hs.DiskStats = diskStats

// Getting the disk stats for the allocation directory
usage, err := disk.Usage(h.allocDir)
if err != nil {
return fmt.Errorf("failed to find disk usage of alloc_dir %q: %v", h.allocDir, err)
h.logger.Error("failed to find disk usage of alloc", "alloc_dir", h.allocDir, "error", err)
hs.AllocDirStats = &DiskStats{}
} else {
hs.AllocDirStats = h.toDiskStats(usage, nil)
}
hs.AllocDirStats = h.toDiskStats(usage, nil)

// Collect devices stats
deviceStats := h.collectDeviceGroupStats()
hs.DeviceStats = deviceStats
Expand Down