openconfig / gnmic

gNMIc is a gNMI CLI client and collector
https://gnmic.openconfig.net
Apache License 2.0
171 stars 55 forks source link

heartbeat flag ignored when using sample mode #340

Closed amamory closed 8 months ago

amamory commented 8 months ago

It seems that the heartbeat flag is ignored when using sample mode. Consider this example:

gnmic -a localhost:50051  --insecure subscribe --path "minimal:cont/counter0" --mode stream --stream-mode sample --sample-interval 5s --heartbeat-interval 1s

And this is what i get in the server side. The heartbeat is not present.

Subscribe request msg:
{
 "subscribe": {
  "subscription": [
   {
    "path": {
     "elem": [
      {
       "name": "minimal:cont"
      },
      {
       "name": "counter0"
      }
     ]
    },
    "mode": 2,
    "sample_interval": "5000000000"
   }
  ]
 }
}

If I do a similar request, but in the on_change mode:

gnmic -a localhost:50051  --insecure subscribe --path "minimal:cont/counter0" --mode stream --stream-mode on_change --heartbeat-interval 1s

Then i see the heartbeat flag as expected at the server side.

Subscribe request msg:
{
 "subscribe": {
  "subscription": [
   {
    "path": {
     "elem": [
      {
       "name": "minimal:cont"
      },
      {
       "name": "counter0"
      }
     ]
    },
    "mode": 1,
    "heartbeat_interval": "1000000000"
   }
  ]
 }
}
karimra commented 8 months ago

In a sample subscription gNMIc includes the heartbeat-interval only if suppress-redundant is true. The spec says the following:

A heartbeat_interval MAY be specified to modify the behavior of suppress_redundant in a sampled subscription. In this case, the target MUST generate one telemetry update per heartbeat interval, regardless of whether the suppress_redundant flag is set to true. This value is specified as an unsigned 64-bit integer in nanoseconds.

I understand it as heartbeat-interval is needed only if suppress-redundant is true.

amamory commented 8 months ago

It sounds like a logical reason. However, from the user perspective who might not be a gNMI spec expert, it would be very nice to see an error/warning message. I suppose it's a minor effort.