cisco-ie / pipeline-gnmi

A Model-Driven Telemetry collector based on the open-source tool pipeline
Apache License 2.0
32 stars 6 forks source link

TestCodecGPBBasic fails #11

Open remingtonc opened 5 years ago

remingtonc commented 5 years ago

TestCodecGPBBasic fails due to different produced and verification data. Produced data includes data_gpb:<nil> and data_gpbkv:[] whereas the verification data does not include these fields. This is likely due to the samples being compared directly from pure JSON as compared to serializing via Telemetry proto which does contain these fields.

569,571c569
< collection_end_time:1.471794103839e+12 collection_id:111814 collection_start_time:1.471794103827e+12 data_gpb:<nil> data_gpbkv:[] encoding_path:Cisco-IOS-XR-wdsysmon-fd-oper:system-monitoring/cpu-utilization msg_timestamp:1.471794103827e+12 node_id_str:uut subscription_id_str:test]
---
> collection_end_time:1.471794103839e+12 collection_id:111814 collection_start_time:1.471794103827e+12 encoding_path:Cisco-IOS-XR-wdsysmon-fd-oper:system-monitoring/cpu-utilization msg_timestamp:1.471794103827e+12 node_id_str:uut subscription_id_str:test]

Trying to determine which should be correct...

remingtonc commented 5 years ago

This test passes in the original bigmuddy repo and the testdata and test itself is the same - this is a failure introduced in this repo.

BenderScript commented 5 years ago

Very interesting. Does the old repo test against recent Go versions? Looking at your description this issue seems like something related to JSON tags such asomitempty.

remingtonc commented 5 years ago

This does appear to be due to the updated vendor dependencies. Copying the vendor/ folder from this project to original pipeline repository yields the same failures. The crux of the failure is this inclusion: data_gpb:<nil> data_gpbkv:[] The proto: https://github.com/cisco-ie/pipeline-gnmi/blob/master/vendor/github.com/cisco/bigmuddy-network-telemetry-proto/proto_go/telemetry.pb.go#L85-L89 Should these fields be in the output?

BenderScript commented 5 years ago

These values should not be there if marshaled correctly. Interestingly these are the only pointers in the struct and although the definition of empty should be straightforward these are very nested custom types.