aristanetworks / cloudvision-python

Python resources and libraries for integrating with Arista's CloudVision platform
Apache License 2.0
25 stars 17 forks source link

Issue with decoder.py unable to handle BGP open send and receive bytes strings #10

Closed lspicer17 closed 2 years ago

lspicer17 commented 2 years ago

When querying BGP neighbor telemetry using the GRPCClient, the __postProcess method in the Decoder class (decoder.py) is unable to handle decoding the BGP OpenSent and OpenReceive bytes strings.

Exception: UnicodeDecodeError: 'utf-8' codec can't decode byte 0xa0 in position 2: invalid start byte

Telemetry path: analytics/Devices/%<deviceId>/versioned-data/routing/bgp/status/vrf/%<vrf>/bgpPeerInfoStatusEntry/%<peer>

An option to resolve this would be to add the replace parameter in the decode method, this will keep the method from raising an exception. It will replace the unrecognized/malformed characters with the official replacement character when decoding. The issues in the data can then be handled outside the GRPCClient.

https://docs.python.org/3/library/codecs.html#codec-base-classes

    def __postProcess(self, b):
        if isinstance(b, bytes):
            # return b.decode('utf-8')
            return b.decode('utf-8', 'replace')  # possible fix
        elif isinstance(b, list):
            return self.decode_array(b)
        elif isinstance(b, (dict, FrozenDict)):
            return self.decode_map(b)
        else:
            return b
cianmcgrath commented 2 years ago

Thanks for opening the issue πŸ˜€
Yes, replace should work for this. We'd previously encountered encoding issues like this before but that was when the connector was only using the ascii codec. We haven't encountered any issues with the new codec until now Will discuss with the team if we want to use replace or ignore here, and I will make a change for this πŸ‘

lspicer17 commented 2 years ago

Awesome, thank you very much!

cianmcgrath commented 2 years ago

Would you at all be able to share with us with the contents of the two fields in the path using something like apish so that we can confirm that the fix works as intended? We so far haven't been able to find any instance of those fields with the data internally to be able to verify the solution rigorously e.g. on your cluster, run /cvpi/tools/apish get -d analytics -p Devices/<deviceId>/versioned-data/routing/bgp/status/vrf/<vrf>/bgpPeerInfoStatusEntry/<peer> -k "OpenSent" -k "OpenReceive" --pretty

lspicer17 commented 2 years ago

Yep, I can share that with you. How would you like me to send that over? I'd prefer not to share it here since those messages technically contain BGP session information (like ASN, router IDs, etc).

cianmcgrath commented 2 years ago

You can email me at mcgrathc@arista.com if that's acceptable.

cianmcgrath commented 2 years ago

Added ignore as part of commit: #a64442dafe8c622e7b566468ad911ad78f9467d1 Should be in the latest release which you can download now πŸ‘

lspicer17 commented 2 years ago

Downloaded and tested, it works! Thank you again for fixing this so quickly!