Skip to content

Commit

Permalink
router: cleanup naming, error reporting and logging.
Browse files Browse the repository at this point in the history
Per reviewer's suggestion:
* Rename Newxyz to Init(), since not returning anything.
* Moved logging to caller side.
* Normalized errors a bit.
  • Loading branch information
jiceatscion committed Oct 23, 2023
1 parent 1c1e4bb commit 9380f5a
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 30 deletions.
28 changes: 14 additions & 14 deletions pkg/private/processmetrics/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,50 +10,50 @@ go_library(
visibility = ["//visibility:public"],
deps = select({
"@io_bazel_rules_go//go/platform:aix": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"@io_bazel_rules_go//go/platform:android": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
"@com_github_prometheus_client_golang//prometheus:go_default_library",
"@com_github_prometheus_procfs//:go_default_library",
],
"@io_bazel_rules_go//go/platform:darwin": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"@io_bazel_rules_go//go/platform:dragonfly": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"@io_bazel_rules_go//go/platform:freebsd": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"@io_bazel_rules_go//go/platform:illumos": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"@io_bazel_rules_go//go/platform:ios": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"@io_bazel_rules_go//go/platform:js": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"@io_bazel_rules_go//go/platform:linux": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
"@com_github_prometheus_client_golang//prometheus:go_default_library",
"@com_github_prometheus_procfs//:go_default_library",
],
"@io_bazel_rules_go//go/platform:netbsd": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"@io_bazel_rules_go//go/platform:openbsd": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"@io_bazel_rules_go//go/platform:plan9": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"@io_bazel_rules_go//go/platform:solaris": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"@io_bazel_rules_go//go/platform:windows": [
"//pkg/log:go_default_library",
"//pkg/private/serrors:go_default_library",
],
"//conditions:default": [],
}),
Expand Down
18 changes: 7 additions & 11 deletions pkg/private/processmetrics/processmetrics_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/procfs"

"github.com/scionproto/scion/pkg/log"
"github.com/scionproto/scion/pkg/private/serrors"
)

var (
Expand Down Expand Up @@ -157,19 +157,18 @@ func (c *procStatCollector) Collect(ch chan<- prometheus.Metric) {
)
}

// NewProcStatCollector creates a new collector for process statistics.
// Init creates a new collector for process statistics.
// The collector exposes those statistics to prometheus and responds
// to scraping requests. Call this only once per process or get an error.
// It is safe to ignore errors from this but prometheus may lack some
// metrics.
func NewProcStatCollector() error {
func Init() error {
me := os.Getpid()
taskPath := filepath.Join(procfs.DefaultMountPoint, strconv.Itoa(me), "task")
taskDir, err := os.Open(taskPath)
if err != nil {
log.Error("NewProcStatCollector: opening /proc/pid/task/ failed",
"pid", me, "error", err)
return err
return serrors.WrapStr("Opening /proc/pid/task/ failed", err,
"pid", me)
}

c := &procStatCollector{
Expand All @@ -179,18 +178,15 @@ func NewProcStatCollector() error {

err = c.updateStat()
if err != nil {
log.Error("NewProcStatCollector: first update failed", "error", err)
// Ditch the broken collector. It won't do anything useful.
return err
return serrors.WrapStr("First update failed", err)
}

// It works. Register it so prometheus milks it.
err = prometheus.Register(c)
if err != nil {
log.Error("NewProcStatCollector", "registration failed", err)
return err
return serrors.WrapStr("Registration failed", err)
}

log.Info("NewProcStatCollector", "collector", c)
return nil
}
7 changes: 3 additions & 4 deletions pkg/private/processmetrics/processmetrics_other.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@
package processmetrics

import (
"github.com/scionproto/scion/pkg/log"
"github.com/scionproto/scion/pkg/private/serrors"
)

func NewProcStatCollector() error {
log.Info("NewProcStatCollector not supported for this platform")
return error("Not supported for this platform)
func Init() error {
return serrors.New("Not supported for this platform")
}
7 changes: 6 additions & 1 deletion router/dataplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -2521,7 +2521,12 @@ func (d *DataPlane) initMetrics() {

// Start our custom /proc/pid/stat collector to export iowait time and
// (in the future) other process-wide metrics that prometheus does not.
_ = processmetrics.NewProcStatCollector()
err := processmetrics.Init()

// we can live without these metrics. Just log the error.
if err != nil {
log.Error("Could not initialize processmetrics", "err", err)
}
}

func initInterfaceMetrics(metrics *Metrics, labels prometheus.Labels) interfaceMetrics {
Expand Down

0 comments on commit 9380f5a

Please sign in to comment.