aperture icon indicating copy to clipboard operation
aperture copied to clipboard

aperture-js: Use binary serialization for the aperture.check_response attribute

Open krdln opened this issue 1 year ago • 1 comments

Json serialization of timestamps has some issues (they're serialized as objects and not strings, as per protojson format) and @harjotgill created a workaround for that, which converts all timestamp fields manually.

https://github.com/fluxninja/aperture/blob/bce96d9ec3e3792358774492f7e825c38dc633a6/sdks/aperture-js/sdk/flow.ts#L78-L108

An alternative workaround would be to utilize binary serialization instead of json. We can do it because metricsprocessor accepts aperture.check_response as json but also as base64-encoded protowire. This could even result in a performance improvement as a side-effect.

Note: This requires swapping out the generator, as the current one doesn't provide anything like serializeBinary / encode, etc.

Note: If this requires some more effort, let's stay with current workaround.

krdln avatar Sep 20 '23 11:09 krdln

Generated new typescript files from proto using https://www.npmjs.com/package/grpc_tools_node_protoc_ts?activeTab=readme#how-to-use Currently have a problem with importing those added files - code pushed to branch js_new_proto_gen. Leaving this task for now.

DariaKunoichi avatar Sep 20 '23 21:09 DariaKunoichi