Issues with auxiliary_metadata and tools which output JSON

17 views
Skip to first unread message

Jeremiah Bonney

unread,
Mar 8, 2021, 6:45:31 PM3/8/21
to Remote Execution APIs Working Group
Hello y’all,

Wanted to bring up a recent incompatibility that came up while working on the buildgrid/buildbox ecosystem regarding the auxiliary_metadata field in ExecutedActionMetadata. We created a custom proto message to capture ‘getrusage’ information, execution_stats.proto, and started populating that from our workers. This worked great, until some of our client tools started to fetch ActionResults with this metadata populated.

We make use of several client tools, both ones we maintain and ones using grpc_cli + reflection, which consume protobuf but output JSON. These tools are used for debugging purposes as well as downloading results of executions for consumption by other systems that don’t use protobuf. Once the custom proto started getting populated, however, these tools started failing.

Turns out, the protobuf to JSON conversion has the requirement that it must have full references to whatever message is populated in the Any field and will fail if it doesn’t, at least using the C++ conversion utilities. This is different behavior than our tools which don’t do this conversion, and caught us off guard.

This seems like an unfortunate bit of incompatibility to me. It means client tools which fetch ActionResults and spit out JSON won’t work across server implementations that, as recommended by the spec, populate auxiliary_metadata with their own custom messages. There are ways to get around this (drop the field entirely, use reflection and hope the implementation you’re using supports it, etc), but it’s a new and unadvertised stumbling block.

What do people think is the best way to deal with this kind of issue? I’ve listed a few ideas that came up when discussing this with others, just to throw some stuff out there:

  • Replace this field with a set of key/value pairs, similar to what’s done for Platform Properties. That would allow implementation specific metrics without requiring clients needing to understand the implementation specific proto.
  • Upload the messages to CAS and populate auxiliary_metadata with their digests instead. This allows any client to be able to convert the entire ActionResponse to json, while implementation specific clients would know how to fetch the extra metadata.

  • Expect any clients using JSON to strip off this field before doing the conversion. This makes tools like grpc_cli incompatible which seems not ideal, but could work for more specific tools. 


Thanks!
- Jeremiah

Ed Schouten

unread,
Mar 9, 2021, 7:55:59 AM3/9/21
to Jeremiah Bonney, Remote Execution APIs Working Group
Hi Jeremiah,

Op di 9 mrt. 2021 om 00:45 schreef Jeremiah Bonney <jjbo...@gmail.com>:
> Wanted to bring up a recent incompatibility that came up while working on the buildgrid/buildbox ecosystem regarding the auxiliary_metadata field in ExecutedActionMetadata. We created a custom proto message to capture ‘getrusage’ information, execution_stats.proto, and started populating that from our workers.

Nice! Buildbarn has been using a similar message for this as well:

https://github.com/buildbarn/bb-remote-execution/blob/master/pkg/proto/resourceusage/resourceusage.proto#L39-L85

Be sure to reach out if you feel like converging on a common message
type for this.

> Turns out, the protobuf to JSON conversion has the requirement that it must have full references to whatever message is populated in the Any field and will fail if it doesn’t, at least using the C++ conversion utilities.
>
> [...]
>
> What do people think is the best way to deal with this kind of issue?

What happens if you provide a custom TypeResolver that resolves all
unknown types to google.protobuf.Empty? Because google.protobuf.Empty
is empty, it should by definition act as a base type for all messages.

--
Ed Schouten <e...@nuxi.nl>
Reply all
Reply to author
Forward
0 new messages