openconfig / gnmi-gateway

A modular, distributed, and highly available service for modern network telemetry via OpenConfig and gNMI
Apache License 2.0
134 stars 32 forks source link

Support for Path.Origin field in Subscribe request #18

Closed tardoe closed 3 years ago

tardoe commented 3 years ago

Some network vendors (e.g. Arista) have implemented support for non-OpenConfig models via GNMI utilising the Origin parameter on a Path.

I can see some comments among the code (e.g. https://github.com/openconfig/gnmi-gateway/blob/1628cd9de05af10c6662084c7d4e34f639ec741c/gateway/server/server.go#L168) indicating a future desire to support this behaviour.

Before I submit a PR, is there any particular desire for this to be supported and in any particular manner?

If/When I do submit a PR, this issue can serve as a reference.

colinmcintosh commented 3 years ago

Hi @tardoe! We do desire to support Origin where possible in gnmi-gateway and there should already be some partial support for it built-in. To clarify, are you looking at including the Origin field with the SubscribeRequest from gnmi-gateway to your targets or are you trying to include the Origin field in a SubscribeRequest being sent to gnmi-gateway?

The first case should be fully supported by the gnmi-gateway connection manager. Only the json Target Loader accepts the Origin field but the simple Target Loader or a custom one could accept it as well.

For the second case, if you are sending a SubscribeRequest to gnmi-gateway with an Origin field set then the Origin field will be ignored as you linked to in the subscribe server code; as it currently stands you should receive all matching paths regardless of the Origin field's value. I did test this out with a subscription to my instance of gnmi-gateway for eos_native paths as an example and was able to receive data from that origin.

tardoe commented 3 years ago

Thanks @colinmcintosh

I have a local patch to support the Origin prefix in the Simple TargetLoader but I was unsuccessful in testing this - the origin doesn’t appear in the SubscribeRequest going to the device.

I can push this to a branch and link it here for you to see what I may be missing.

colinmcintosh commented 3 years ago

I took a shot at adding Origin to the simple target loader and it looks like it's working for me when I test it against one of my lab devices; unit tests look good when checking the Origin in the SubscribeRequest as well.

Would you be able to pull the latest commit from github.com/colinmcintosh/gnmi-gateway@release (see PR #20) and check if this change is working for you? I'd also be happy to help troubleshoot your patch if you'd rather post the code and link me to that.

colinmcintosh commented 3 years ago

Merged the PR for this. Closing out the issue.