-
Notifications
You must be signed in to change notification settings - Fork 28
Conversation
zhangtianyang
commented
Aug 15, 2018
- Added a goroutine to periodically collect AP & system states.
- Added a Prometheus exporter for link022 to export states.
one update. 2. Fix a bug in using context.
2. support array type metric value
fixed a bug caused by misusing append function. fixed a bug in generating metric name.
fixed a bug in reading cpu usage file
by forgeting reset prometheus collector after every test.
Codecov Report
@@ Coverage Diff @@
## master #81 +/- ##
==========================================
- Coverage 63% 59.48% -3.53%
==========================================
Files 11 14 +3
Lines 638 1049 +411
==========================================
+ Hits 402 624 +222
- Misses 167 329 +162
- Partials 69 96 +27
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All new files need license headers.
agent/monitoring/monitoring.go
Outdated
const ( | ||
statesUpdateDelay = 15 * time.Second | ||
systemClockTick = 100 | ||
physicalMemoryPath = "/access-points/access-point[hostname=link022-pi-ap]/system/memory/state/physical" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not hard code host name here. It is not always link022-pi-ap. Same for the rest of paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
agent/monitoring/monitoring.go
Outdated
p := strings.Replace(selfMemPath, "$pid", spid, 1) | ||
pbPath, err = xpath.ToGNMIPath(p) | ||
if err != nil { | ||
log.Errorf("convert %v to GNMI path failed: %v", p, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to log this, since it is already logged in caller function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
agent/monitoring/monitoring.go
Outdated
} | ||
selfMemory, err := strconv.ParseInt(match[1], 10, 64) | ||
if err != nil { | ||
log.Errorf("failed convert string to int: %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
} | ||
stateOpt = gnmi.GNXIStateOptGenerator(pbPath, uint64(selfMemory*1024), gnmi.InternalUpdateState) | ||
if err = s.InternalUpdate(stateOpt); err != nil { | ||
log.Errorf("update state failed: %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
log.Errorf("failed open %v: %v", filePath, err) | ||
return err | ||
} | ||
cpuStr0 := strings.Split(string(b0), " ") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better to use regex instead of doing spiting and using hard-coded index.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file simply contains 52 numbers. Their positions are fixed. So it's better to find them by index.
agent/monitoring/monitoring.go
Outdated
p := strings.Replace(channelPath, "$id", phyIDStr, 1) | ||
pbPath, err := xpath.ToGNMIPath(p) | ||
if err != nil { | ||
log.Errorf("convert %v to GNMI path failed: %v", p, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to log error before return. Same for others.
|
||
### Prerequisites | ||
|
||
Dolnload entire Link022 repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Download
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -0,0 +1,83 @@ | |||
# OpenConfig Telemetry Exporter (Prometheus) | |||
|
|||
This directory contains a exporter that periodically collects AP's info via gNMI and exposes converted metrics to a web page. Prometheus can monitor AP's status by scrape that web page. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an exporter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -0,0 +1,83 @@ | |||
# OpenConfig Telemetry Exporter (Prometheus) | |||
|
|||
This directory contains a exporter that periodically collects AP's info via gNMI and exposes converted metrics to a web page. Prometheus can monitor AP's status by scrape that web page. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by scraping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
||
var ( | ||
targetAddr = flag.String("target_addr", "localhost:10161", "The target address in the format of host:port") | ||
targetName = flag.String("target_name", "hostname.com", "The target name use to verify the hostname returned by TLS handshake") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
target name used to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Corrected some typos.