[dataset] Add dataset "system uptime" into non-db client. #52
Conversation
Signed-off-by: Yong Zhao <yozhao@microsoft.com>
Signed-off-by: Yong Zhao <yozhao@microsoft.com>
Signed-off-by: Yong Zhao <yozhao@microsoft.com>
sonic_data_client/non_db_client.go
Outdated
| @@ -63,6 +63,10 @@ var ( | |||
| path: []string{"OTHERS", "proc", "stat"}, | |||
| getFunc: dataGetFunc(getProcStat), | |||
| }, | |||
| { // Get host uptime | |||
| path: []string{"OTHERS", "platform", "sysuptime"}, | |||
There was a problem hiding this comment.
Shouldn't the path be proc/uptime?
The other proc based paths have proc prefix. This is in the same category with them.
There was a problem hiding this comment.
I think the dataset system uptime is depended on platform, just like the dataset CPU utilization at line 43. @pra-moh any suggestions?
There was a problem hiding this comment.
@yozhao101 "/proc/uptime" should not be dependent on platform. Do you have any reference here?
There was a problem hiding this comment.
I referred the dataset of CPU utilization. Did you mean we should put system uptime under the query of proc, right?
There was a problem hiding this comment.
yes let's put it as /proc/uptime prefix
There was a problem hiding this comment.
updated! Please have a review!
sonic_data_client/non_db_client.go
Outdated
| func getSysUptime() ([]byte, error) { | ||
| timeInfo, _ := linuxproc.ReadUptime("/proc/uptime") | ||
| uptime := sysUptime{} | ||
| uptime.Total = uint64(timeInfo.Total) |
There was a problem hiding this comment.
I think it can simply marshal and return whatever linuxproc.ReadUptime gives.
Do we need to convert this to another struct?
There was a problem hiding this comment.
Thelinuxproc.ReadUptime("/proc/uptime") will return a struct which contains two float values: one value is system uptime, the other is the idle time which all CPUs are in the idle state. I sent an email to Pradyna and Hui. Hui confirmed that currently we are only interested in system uptime and do not care about idle time. Also we need convert the float to int.
There was a problem hiding this comment.
@yozhao101 can you please show sample output of linuxproc.ReadUptime("/proc/uptime")? I tend to agree with Murat here because we can always discard from collector instead here to avoid any regression.
There was a problem hiding this comment.
@pra-moh The following is the sample output of linuxproc.ReadUptime("/proc/uptime"). Please review it.
admin@str-a7050-acs-3:~$ ./gnmi_cli -client_types=gnmi -a localhost:8080 -t OTHERS -logtostderr -insecure -qt p -pi 10s -q platform/sysuptime
{
"OTHERS": {
"platform": {
"sysuptime": "{\"total\":2986.02,\"idle\":10512.98}"
}
}
}
{
"OTHERS": {
"platform": {
"sysuptime": "{\"total\":2996.02,\"idle\":10547.75}"
}
}
}
{
"OTHERS": {
"platform": {
"sysuptime": "{\"total\":3006.02,\"idle\":10583.74}"
}
}
}
There was a problem hiding this comment.
Thanks Yong. Let's return total and idle values as they are. As Murat suggested just marshal and return don't do any conversion. We can handle conversion/discarding idle at collector side
There was a problem hiding this comment.
Sure, I will update.
…f CPU utilization. Signed-off-by: Yong Zhao <yozhao@microsoft.com>
long CPUs are in the idle state. Signed-off-by: Yong Zhao <yozhao@microsoft.com>
Signed-off-by: Yong Zhao <yozhao@microsoft.com>
sonic_data_client/non_db_client.go
Outdated
| @@ -43,6 +43,10 @@ var ( | |||
| path: []string{"OTHERS", "platform", "cpu"}, | |||
| getFunc: dataGetFunc(getCpuUtil), | |||
| }, | |||
| { // Get host uptime | |||
| path: []string{"OTHERS", "proc", "sysuptime"}, | |||
There was a problem hiding this comment.
I think it is better to name this query proc/uptime (e.g. drop sys). The other queries in this category follow a particular convention where query path proc/xyz directly maps to file system /proc/xyz.
There was a problem hiding this comment.
Great suggestion! Updated.
Signed-off-by: Yong Zhao <yozhao@microsoft.com>
Signed-off-by: Yong Zhao <yozhao@microsoft.com>
Signed-off-by: Yong Zhao <yozhao@microsoft.com>
Signed-off-by: Yong Zhao yozhao@microsoft.com
Motivation of this PR:
This PR aims to enable the streaming telemetry container to stream out the
system uptimeof SONiC Host. This dataset is added into non-database client since it is a kind of value which will be refreshed periodically.How can I do that?
I follow the example of dataset in non-database client such as
meminfoto do the implementation. The data source ofsystem uptimeis from the file/proc/uptimeon the SONiC host.We can use the command
./gnmi_cli -client_types=gnmi -a <DuT_IP>:8080 -t OTHERS -logtostderr -insecure -qt p -pi 10s -q proc/uptimeto query thesystem uptimeevery 10 seconds.admin@str-a7050-acs-3:~$ ./gnmi_cli -client_types=gnmi -a localhost:8080 -t OTHERS -logtostderr -insecure -qt p -pi 10s -q proc/uptime
{
"OTHERS": {
"proc": {
"uptime": "{"total":314.74,"idle":981.27}"
}
}
}
{
"OTHERS": {
"proc": {
"uptime": "{"total":324.74,"idle":1016.85}"
}
}
}