bazelbuild / remote-apis

An API for caching and execution of actions on a remote system.
Apache License 2.0
341 stars 119 forks source link

Allow abitrary tagging in RequestMetadata #304

Open sluongng opened 4 months ago

sluongng commented 4 months ago

Issue Description

Background

This issue is a follow-up to Bazel Issue #21332.

Summary

In some setups, it is beneficial to provide metadata about the target being built to the Remote Build Execution (RBE) service. This metadata can help in identifying the "ownership" of a specific action, target, artifact, or test. If there are anomalies during the build, the build service can use this information to alert the appropriate on-call rotation. For example, in Buck2, oncall() annotations at the package level in BUCK files serve this purpose (example).

Use Cases

  1. Ownership Tagging: Identifying the owner of a build action to help with on-call rotations.
  2. Cost Attribution: Tagging for Remote Cache and RBE cost attribution based on team, service, programming language, or relevant categories.

Proposal

To support these use cases, I propose to add the following to the specification:

--- a/build/bazel/remote/execution/v2/remote_execution.proto
+++ b/build/bazel/remote/execution/v2/remote_execution.proto
@@ -2118,4 +2118,7 @@ message RequestMetadata {
   // There is no expectation that this value will have any particular structure,
   // or equality across invocations, though some client tools may offer these guarantees.
   string configuration_id = 7;
+
+  // Multiple group identifiers for the target that produced this action.
+  repeated string target_tags = 8;
 }

By adding target_tags as a repeated string field in the RequestMetadata message, multiple group identifiers for the target can be specified.

ColdrickSotK commented 1 month ago

I'd also be interested in a generic repeated field in RequestMetadata like this proposal for similar reasons around helping with request attribution.

Would you like me to push a PR proposing this, or are you planning on doing that sometime?

sluongng commented 1 month ago

I was hoping to get some feedback from current maintainers/contributors first to see if this is the best approach. But forgot to bring this PR up in recent meetings.

If you want to send a PR, please go ahead. 🤝

bergsieker commented 1 month ago

I've considered adding something like this at several points in the past, so I'm generally supportive. Given that the field is in an optional metadata message, it's important that it not be used for anything functional (e.g., scheduling, cache key, etc.) but it could be quite useful for various metrics and analyses.

One thing to consider is whether simple string tags are sufficient, or whether we might want key-value pairs. Or both.