Write string tags to invocation record

Summary:
Can be useful to have these in error_tags and not just the category key.

Since this requires storing these in ErrorReport it also means these won't be dropped if string tags are added to command errors or daemon errors, although this is not currently used.

Reviewed By: iguridi

Differential Revision: D71156099

fbshipit-source-id: 58f19c7b91399befd7e04315206d24d877c7a768

Author: christolliday

app/buck2_client_ctx/src/subscribers/recorder.rs

@@ -1649,20 +1649,24 @@ fn process_error_report(error: buck2_data::ErrorReport) -> buck2_data::Processed
         Tier::Environment => ENVIRONMENT.to_owned(),
         Tier::Input => INPUT.to_owned(),
     };
+    let tags = error
+        .tags
+        .iter()
+        .copied()
+        .filter_map(|v| buck2_data::error::ErrorTag::try_from(v).ok())
+        .map(|t| t.as_str_name().to_owned());
+
+    let string_tags = error.string_tags.iter().map(|t| t.tag.clone());
+    let tags = tags.chain(string_tags).collect();
+
     buck2_data::ProcessedErrorReport {
         tier: None,
         message: error.message,
         telemetry_message: error.telemetry_message,
         source_location: error
             .source_location
             .map(|s| SourceLocation::from(s).to_string()),
-        tags: error
-            .tags
-            .iter()
-            .copied()
-            .filter_map(|v| buck2_data::error::ErrorTag::try_from(v).ok())
-            .map(|t| t.as_str_name().to_owned())
-            .collect(),
+        tags,
         best_tag: Some(best_tag),
         sub_error_categories: error.sub_error_categories,
         category_key: error.category_key,

app/buck2_data/data.proto

@@ -2400,6 +2400,9 @@ message ErrorReport {
     optional string type_name = 2;
     optional uint32 source_line = 3;
   }
+  message StringTag {
+    string tag = 1;
+  }
 
   reserved 1, 2;
   // The error message that is shown to users on the CLI
@@ -2416,6 +2419,7 @@ message ErrorReport {
   repeated buck.data.error.ErrorTag tags = 6;
   repeated string sub_error_categories = 7;
   optional string category_key = 8;
+  repeated StringTag string_tags = 9;
 }
 
 // Identical to `ErrorReport`, but with the tags converted to strings.

app/buck2_error/src/conversion/report.rs

@@ -9,6 +9,8 @@
 
 use buck2_data::ErrorReport;
 
+use crate::context_value::ContextValue;
+use crate::context_value::StringTag;
 use crate::source_location::SourceLocation;
 use crate::ErrorTag;
 
@@ -23,7 +25,8 @@ impl From<ErrorReport> for crate::Error {
             .into_iter()
             .filter_map(|v| ErrorTag::try_from(v).ok())
             .collect();
-        crate::Error::new(
+
+        let mut error = crate::Error::new(
             value.message.clone(),
             *tags.first().unwrap_or(&ErrorTag::Tier0),
             value
@@ -32,7 +35,13 @@ impl From<ErrorReport> for crate::Error {
                 .unwrap_or(SourceLocation::new(file!())),
             None,
         )
-        .tag(tags)
+        .tag(tags);
+
+        for tag in value.string_tags {
+            let tag = ContextValue::StringTag(StringTag { tag: tag.tag });
+            error = error.context(tag);
+        }
+        error
     }
 }
 
@@ -64,12 +73,22 @@ impl From<&crate::Error> for ErrorReport {
         } else {
             vec![]
         };
+        let string_tags = err
+            .iter_context()
+            .filter_map(|kind| match kind {
+                ContextValue::StringTag(val) => Some(buck2_data::error_report::StringTag {
+                    tag: val.tag.clone(),
+                }),
+                _ => None,
+            })
+            .collect();
 
         buck2_data::ErrorReport {
             message,
             telemetry_message,
             source_location: Some(err.source_location().clone().into()),
             tags: err.tags().iter().map(|t| *t as i32).collect(),
+            string_tags,
             sub_error_categories,
             category_key: Some(category_key),
         }

tests/core/build/test_error_categorization.py

@@ -217,7 +217,8 @@ async def test_daemon_crash(buck: Buck, tmp_path: Path) -> None:
     else:
         assert "stream closed because of a broken pipe" in error["message"]
 
-    assert error["tags"] == ["CLIENT_GRPC", "SERVER_PANICKED", "TIER0"]
+    assert error["tags"][0:3] == ["CLIENT_GRPC", "SERVER_PANICKED", "TIER0"]
+    assert error["tags"][3].startswith("crash")
     assert "buckd stderr:\n" in error["message"]
     assert "panicked at" in error["message"]
 
@@ -282,6 +283,8 @@ async def test_daemon_abort(buck: Buck, tmp_path: Path) -> None:
     # TODO dump stack trace on mac and windows
     if is_running_on_linux():
         assert "crash(" in category_key
+        # test string tags are in error_tags
+        assert error["tags"][-1].startswith("crash(")
 
 
 @buck_test()