proposal: deprecate the Listener function for DeviceHandlers

[edaniszewski]

This thought came about when thinking through implementation details about how to support streamed telemetry from juniper devices.

First, a bit of history: the Listener function on the DeviceHandler was initially addd as a first attempt at handling streaming data, where the use case was the sFlow plugin, which was never fully realized and never progressed out of the initial development phase, largely because it was exploratory and we did not/do not currently have a need for sFlow data.

The Listener function worked okay for the initial experimentation with sFlow, but now investigating telemetry streaming for juniper, it doesn't seem ideal. The largest issue being that it ties a "listen" to a single device. So, for the case of a juniper router, this would mean that you would have to consider the router to be the single device and every single piece of data it produces would be readings for the one device.

Instead, it makes sense (in the juniper case) to consider devices to be finer-grained (e.g. a single port on the router), but this isn't possible to do with the current Listener implementation.

Instead of proposing to change it to fit the needs of juniper telemetry streaming, it seems like it should be deprecated, as future streamed data may need to be handled differently.

By deprecating this handler function, it would be left to the plugin author to do a bit more implementation work in setting up the listener function (e.g. as a plugin startup action, or just as a separate goroutine in the main function...) and to cache the data in some in-memory storage, where the devices can then be "read" normally, just performing lookups in the in-memory storage.

It is perhaps not the most ideal nor the most optimized, but I do think that it is the most flexible and future proof and should work for any flavor of streamed data.

Open to thoughts / opinions / discussion / etc.

[MatthewHink]

Looking at the source I think this makes sense. I don't see existing code standing on scheduleListen() other than sdk tests unless I'm missing it.

[edaniszewski]

That sounds right - the only other place that uses it is in the old dev work for the sFlow plugin and in one of the example plugins in the sdk's examples directory

ty for looking through and for the feedback!

[lazypower]

Instead, it makes sense (in the juniper case) to consider devices to be finer-grained (e.g. a single port on the router), but this isn't possible to do with the current Listener implementation.

That was my initial thought as I read through the proposal is that in this context, we presume a device generates a single series of metrics, not 1-N^ metrics, in the case of a switch/router all the ports being the identified "device" makes sense to me, but there's still the case of aggregates across these ports and the uplinks / sfp+ ports linking switches/routers together. Those metrics are just as important as the individual ports.

Instead of proposing to change it to fit the needs of juniper telemetry streaming, it seems like it should be deprecated, as future streamed data may need to be handled differently.

I agree. Greenfield on this seems like the right approach vs trying to retrofit an experimental code block.

By deprecating this handler function, it would be left to the plugin author to do a bit more implementation work in setting up the listener function (e.g. as a plugin startup action, or just as a separate goroutine in the main function...) and to cache the data in some in-memory storage, where the devices can then be "read" normally, just performing lookups in the in-memory storage.

I'm not real sure what this does for us? I thought the intention was to have the routers configured to push data to this plugin, the listen() function can do any number of things here, in-memory caching for subsequent reads, or use of memcached/redis/flatfile/boltdb

in any case, i think we're on the right path and I agree that deprecating the older functionality is a side-effect of doing a newer/better implementation for the streaming metrics, which could then be used to issue an update to the SDK/Plugin code and generate new examples using the newer approach.

[edaniszewski]

this has been completed in the above linked PR