From 2509790e998694998c9aaae09b3eab4046b1611f Mon Sep 17 00:00:00 2001 From: Colin Moy Date: Tue, 25 Aug 2026 20:24:17 +0000 Subject: [PATCH 1/5] feat(generator): handle Any fields with Value or Struct --- generator/internal/discovery_to_proto.cc | 46 +++++++++++-- generator/internal/discovery_to_proto_test.cc | 65 ++++++++++++++++++- generator/internal/discovery_type_vertex.cc | 40 ++++++++++-- .../internal/discovery_type_vertex_test.cc | 52 ++++++++++++++- 4 files changed, 189 insertions(+), 14 deletions(-) diff --git a/generator/internal/discovery_to_proto.cc b/generator/internal/discovery_to_proto.cc index 9dfd24ef3357d..0baf70a01b4c3 100644 --- a/generator/internal/discovery_to_proto.cc +++ b/generator/internal/discovery_to_proto.cc @@ -108,7 +108,10 @@ StatusOr GetImportForProtobufType( std::string const& protobuf_type) { static auto const* const kProtobufTypeImports = new std::unordered_map{ - {"google.protobuf.Any", "google/protobuf/any.proto"}}; + {"google.protobuf.Any", "google/protobuf/any.proto"}, + {"google.protobuf.ListValue", "google/protobuf/struct.proto"}, + {"google.protobuf.Struct", "google/protobuf/struct.proto"}, + {"google.protobuf.Value", "google/protobuf/struct.proto"}}; auto iter = kProtobufTypeImports->find(protobuf_type); if (iter == kProtobufTypeImports->end()) { @@ -353,23 +356,54 @@ std::set FindAllTypesToImport(nlohmann::json const& json) { auto const* current = worklist.back(); worklist.pop_back(); - if (current->contains("type") && (*current)["type"] == "any") { - types_to_import.insert("google.protobuf.Any"); - } if (current->contains("$ref")) { types_to_import.insert((*current)["$ref"]); } + if (current->contains("format")) { + std::string const format = (*current)["format"]; + if (absl::StartsWith(format, "google.protobuf.")) { + types_to_import.insert(format); + // This node is resolved as a protobuf message (e.g. google.protobuf.Any + // on Status.details items), so do not traverse into its internal + // additionalProperties or properties. + continue; + } + } + + if (current->contains("type") && (*current)["type"] == "any") { + types_to_import.insert("google.protobuf.Value"); + } + if (IsDiscoveryNestedType(*current) || current->contains("properties")) { for (auto const& f : (*current)["properties"]) { worklist.push_back(&f); } } + if (IsDiscoveryArrayType(*current)) { - worklist.push_back(&(*current)["items"]); + auto const& items = (*current)["items"]; + if (items.contains("type") && items["type"] == "object" && + items.contains("additionalProperties") && + items["additionalProperties"].value("type", "") == "any" && + !items.contains("format") && + !items["additionalProperties"].contains("format")) { + types_to_import.insert("google.protobuf.Struct"); + } else { + worklist.push_back(&items); + } } + if (IsDiscoveryMapType(*current)) { - worklist.push_back(&(*current)["additionalProperties"]); + auto const& additional_properties = (*current)["additionalProperties"]; + if (additional_properties.contains("type") && + additional_properties["type"] == "any" && + !additional_properties.contains("format") && + !current->contains("format")) { + types_to_import.insert("google.protobuf.Struct"); + } else { + worklist.push_back(&additional_properties); + } } } diff --git a/generator/internal/discovery_to_proto_test.cc b/generator/internal/discovery_to_proto_test.cc index 2af64825e062e..99548453cda32 100644 --- a/generator/internal/discovery_to_proto_test.cc +++ b/generator/internal/discovery_to_proto_test.cc @@ -1276,7 +1276,7 @@ TEST(FindAllTypesToImportTest, SimpleAnyField) { auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false); ASSERT_TRUE(parsed_json.is_object()); auto result = FindAllTypesToImport(parsed_json); - EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Any")); + EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Value")); } TEST(FindAllTypesToImportTest, MultipleSimpleRefFields) { @@ -1345,7 +1345,7 @@ TEST(FindAllTypesToImportTest, ArrayRefAnyFields) { auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false); ASSERT_TRUE(parsed_json.is_object()); auto result = FindAllTypesToImport(parsed_json); - EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Any", "Bar")); + EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Struct", "Bar")); } TEST(FindAllTypesToImportTest, MapRefFields) { @@ -1384,12 +1384,72 @@ TEST(FindAllTypesToImportTest, MapAnyFields) { } })"""; + auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false); + ASSERT_TRUE(parsed_json.is_object()); + auto result = FindAllTypesToImport(parsed_json); + EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Struct")); +} + +TEST(FindAllTypesToImportTest, StatusDetailsField) { + auto constexpr kTypeJson = R"""({ + "properties": { + "details": { + "type": "array", + "items": { + "format": "google.protobuf.Any", + "type": "object", + "additionalProperties": { + "type": "any" + } + } + } + } +})"""; + auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false); ASSERT_TRUE(parsed_json.is_object()); auto result = FindAllTypesToImport(parsed_json); EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Any")); } +TEST(FindAllTypesToImportTest, ArrayAnyWithFormat) { + auto constexpr kTypeJson = R"""({ + "properties": { + "field_1": { + "type": "array", + "items": { + "type": "any", + "format": "google.protobuf.Struct" + } + } + } +})"""; + + auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false); + ASSERT_TRUE(parsed_json.is_object()); + auto result = FindAllTypesToImport(parsed_json); + EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Struct")); +} + +TEST(FindAllTypesToImportTest, MapAnyWithFormat) { + auto constexpr kTypeJson = R"""({ + "properties": { + "map_1": { + "type": "object", + "additionalProperties": { + "type": "any", + "format": "google.protobuf.Value" + } + } + } +})"""; + + auto const parsed_json = nlohmann::json::parse(kTypeJson, nullptr, false); + ASSERT_TRUE(parsed_json.is_object()); + auto result = FindAllTypesToImport(parsed_json); + EXPECT_THAT(result, UnorderedElementsAre("google.protobuf.Value")); +} + TEST(FindAllTypesToImportTest, SingleNestedRefField) { auto constexpr kTypeJson = R"""({ "properties": { @@ -2586,6 +2646,7 @@ TEST_F(AssignResourcesAndTypesToFilesTest, ResourceAndCommonFilesWithImports) { "properties": { "permissions": { "items": { + "format": "google.protobuf.Any", "type": "object", "additionalProperties": { "type": "any" diff --git a/generator/internal/discovery_type_vertex.cc b/generator/internal/discovery_type_vertex.cc index 82a574dfa4936..9568d6d662138 100644 --- a/generator/internal/discovery_type_vertex.cc +++ b/generator/internal/discovery_type_vertex.cc @@ -110,8 +110,13 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, } if (type == "any") { - return TypeInfo{"google.protobuf.Any", compare_package_name, - properties_for_synthesis, false, false}; + if (v.contains("format")) { + type = v["format"]; + } else { + type = "google.protobuf.Value"; + } + return TypeInfo{type, compare_package_name, properties_for_synthesis, false, + false}; } if (type == "object" && @@ -146,7 +151,14 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, properties_for_synthesis = &additional_properties; is_message = true; } else if (map_type == "any") { - return TypeInfo{"google.protobuf.Struct", compare_package_name, + if (additional_properties.contains("format")) { + map_type = additional_properties["format"]; + } else if (v.contains("format")) { + map_type = v["format"]; + } else { + map_type = "google.protobuf.Struct"; + } + return TypeInfo{map_type, compare_package_name, properties_for_synthesis, true, is_message}; } else { return internal::InvalidArgumentError( @@ -181,13 +193,26 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, scalar_type = CheckForScalarType(items); if (scalar_type) { type = *scalar_type; + } else if (type == "any") { + if (items.contains("format")) { + type = items["format"]; + } else { + type = "google.protobuf.Value"; + } + return TypeInfo{type, compare_package_name, nullptr, false, false}; } else if (type == "object" && items.contains("properties")) { // Synthesize a nested type for this array. type = CapitalizeFirstLetter(field_name + "Item"); return TypeInfo{type, compare_package_name, &items, false, true}; } else if (type == "object" && items.contains("additionalProperties") && (items["additionalProperties"]).value("type", "") == "any") { - type = "google.protobuf.Any"; + if (items.contains("format")) { + type = items["format"]; + } else if (items["additionalProperties"].contains("format")) { + type = items["additionalProperties"]["format"]; + } else { + type = "google.protobuf.Struct"; + } return TypeInfo{type, compare_package_name, nullptr, false, false}; } else { return internal::InvalidArgumentError( @@ -544,6 +569,13 @@ StatusOr DiscoveryTypeVertex::GetFieldNumber( } if (field_descriptor->name() == field_name && type_name != field_type) { + // Allow migration of google.protobuf.Any to google.protobuf.Struct or + // google.protobuf.Value. + if (absl::StrContains(type_name, "google.protobuf.Any") && + (absl::StrContains(field_type, "google.protobuf.Struct") || + absl::StrContains(field_type, "google.protobuf.Value"))) { + return field_descriptor->number(); + } // Existing field type has changed. This is a breaking change. return internal::InvalidArgumentError(absl::StrFormat( "Message: %s has field: %s whose type has changed " diff --git a/generator/internal/discovery_type_vertex_test.cc b/generator/internal/discovery_type_vertex_test.cc index 45337e1dbec44..eafbc5ade81bc 100644 --- a/generator/internal/discovery_type_vertex_test.cc +++ b/generator/internal/discovery_type_vertex_test.cc @@ -220,7 +220,16 @@ INSTANTIATE_TEST_SUITE_P( DetermineTypesSuccess{"string", R"""({"type":"string"})""", "string", true, false, false, false}, DetermineTypesSuccess{"any", R"""({"type":"any"})""", - "google.protobuf.Any", true, false, false, false}, + "google.protobuf.Value", true, false, false, + false}, + DetermineTypesSuccess{ + "any_with_format", + R"""({"type":"any","format":"google.protobuf.Value"})""", + "google.protobuf.Value", true, false, false, false}, + DetermineTypesSuccess{ + "any_with_struct_format", + R"""({"type":"any","format":"google.protobuf.Struct"})""", + "google.protobuf.Struct", true, false, false, false}, DetermineTypesSuccess{"boolean", R"""({"type":"boolean"})""", "bool", true, false, false, false}, DetermineTypesSuccess{"integer_no_format", R"""({"type":"integer"})""", @@ -246,6 +255,21 @@ INSTANTIATE_TEST_SUITE_P( DetermineTypesSuccess{ "array_any", R"""({"type":"array","items":{"type":"object","additionalProperties":{"type":"any"}}})""", + "google.protobuf.Struct", true, false, false, false}, + DetermineTypesSuccess{ + "array_any_with_format", + R"""({"type":"array","items":{"type":"object","additionalProperties":{"type":"any","format":"google.protobuf.Value"}}})""", + "google.protobuf.Value", true, false, false, false}, + DetermineTypesSuccess{ + "array_items_any", R"""({"type":"array","items":{"type":"any"}})""", + "google.protobuf.Value", true, false, false, false}, + DetermineTypesSuccess{ + "array_items_any_with_format", + R"""({"type":"array","items":{"type":"any","format":"google.protobuf.Struct"}})""", + "google.protobuf.Struct", true, false, false, false}, + DetermineTypesSuccess{ + "status_details_any", + R"""({"type":"array","items":{"type":"object","format":"google.protobuf.Any","additionalProperties":{"type":"any"}}})""", "google.protobuf.Any", true, false, false, false}, DetermineTypesSuccess{ "array_nested_message", @@ -266,6 +290,14 @@ INSTANTIATE_TEST_SUITE_P( "any_to_struct", R"""({"type":"object","additionalProperties":{"type":"any"}})""", "google.protobuf.Struct", true, true, false, false}, + DetermineTypesSuccess{ + "map_any_with_format", + R"""({"type":"object","additionalProperties":{"type":"any","format":"google.protobuf.Value"}})""", + "google.protobuf.Value", true, true, false, false}, + DetermineTypesSuccess{ + "map_any_with_outer_format", + R"""({"type":"object","format":"google.protobuf.Value","additionalProperties":{"type":"any"}})""", + "google.protobuf.Value", true, true, false, false}, DetermineTypesSuccess{ "map_nested_message", R"""({"type":"object","additionalProperties":{"type":"object", "properties":{}}})""", @@ -846,6 +878,7 @@ message Bar {} syntax = "proto3"; package generator.test; +import "google/protobuf/any.proto"; import "imported.proto"; message Foo {} @@ -857,6 +890,8 @@ message FieldsOnly { int32 field4 = 4; repeated generator.imported.Bar field5 = 5; map field6 = 6; + google.protobuf.Any field7 = 7; + google.protobuf.Any field8 = 8; } )"""; @@ -878,7 +913,7 @@ message FieldsOnly { ASSERT_STATUS_OK(field_number); EXPECT_THAT(*field_number, Eq(1)); - int const candidate_field_number = 7; + int const candidate_field_number = 9; message_descriptor = file_descriptor->FindMessageTypeByName("FieldsOnly"); ASSERT_THAT(message_descriptor, NotNull()); @@ -923,6 +958,19 @@ message FieldsOnly { ASSERT_STATUS_OK(existing_map_different_package_field_number); EXPECT_THAT(*existing_map_different_package_field_number, Eq(6)); + auto existing_any_to_struct_field_number = + DiscoveryTypeVertex::GetFieldNumber(message_descriptor, "field7", + "google.protobuf.Struct", + candidate_field_number); + ASSERT_STATUS_OK(existing_any_to_struct_field_number); + EXPECT_THAT(*existing_any_to_struct_field_number, Eq(7)); + + auto existing_any_to_value_field_number = DiscoveryTypeVertex::GetFieldNumber( + message_descriptor, "field8", "google.protobuf.Value", + candidate_field_number); + ASSERT_STATUS_OK(existing_any_to_value_field_number); + EXPECT_THAT(*existing_any_to_value_field_number, Eq(8)); + auto field_type_changed = DiscoveryTypeVertex::GetFieldNumber( message_descriptor, "field6", "map", candidate_field_number); From 214eb62c3c425c6e361d6d14f617ba2345f69438 Mon Sep 17 00:00:00 2001 From: Colin Moy Date: Thu, 27 Aug 2026 19:42:17 +0000 Subject: [PATCH 2/5] feat(generator): add debug_redact to key fields and enable redact in DebugString --- generator/internal/discovery_type_vertex.cc | 20 +++ .../internal/discovery_type_vertex_test.cc | 128 ++++++++++++++++++ .../cloud/internal/debug_string_protobuf.cc | 1 + .../internal/debug_string_protobuf_test.cc | 51 ++++++- 4 files changed, 199 insertions(+), 1 deletion(-) diff --git a/generator/internal/discovery_type_vertex.cc b/generator/internal/discovery_type_vertex.cc index 9568d6d662138..f6666f69e3cae 100644 --- a/generator/internal/discovery_type_vertex.cc +++ b/generator/internal/discovery_type_vertex.cc @@ -17,6 +17,8 @@ #include "google/cloud/internal/algorithm.h" #include "google/cloud/internal/make_status.h" #include "google/cloud/log.h" +#include "absl/strings/ascii.h" +#include "absl/strings/match.h" #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" #include "absl/strings/str_replace.h" @@ -43,6 +45,16 @@ std::optional CheckForScalarType(nlohmann::json const& j) { return std::nullopt; } +bool IsStringOrBytes(nlohmann::json const& field_json) { + std::string const type = field_json.value("type", ""); + if (type == "string" || type == "bytes") return true; + if (type == "array" && field_json.contains("items")) { + std::string const item_type = field_json["items"].value("type", ""); + if (item_type == "string" || item_type == "bytes") return true; + } + return false; +} + } // namespace DiscoveryTypeVertex::DiscoveryTypeVertex( @@ -495,6 +507,12 @@ std::string DiscoveryTypeVertex::FormatFieldOptions( absl::StrCat("\"", field_name, "\"")); } + if (IsStringOrBytes(field_json) && + (absl::StrContains(field_name, "key") || + absl::StrContains(absl::AsciiStrToLower(json_field_name), "key"))) { + field_options.emplace_back("debug_redact", "true"); + } + // Discovery doc defined field names that are not always in strict // camelCase, leading to translation issue between json and protobuf. Thus, // the emitted proto fields need to have their name as it appears in the @@ -512,6 +530,8 @@ std::string DiscoveryTypeVertex::FormatFieldOptions( std::pair const& p) { if (p.first == "json_name") { *s += absl::StrFormat("%s=\"%s\"", p.first, p.second); + } else if (p.first == "debug_redact") { + *s += absl::StrFormat("%s = %s", p.first, p.second); } else { *s += absl::StrFormat("(%s) = %s", p.first, p.second); } diff --git a/generator/internal/discovery_type_vertex_test.cc b/generator/internal/discovery_type_vertex_test.cc index eafbc5ade81bc..62fcb454dcfae 100644 --- a/generator/internal/discovery_type_vertex_test.cc +++ b/generator/internal/discovery_type_vertex_test.cc @@ -182,6 +182,92 @@ TEST(DiscoveryTypeVertexTest, FormatFieldOptionsRequiredIsResource) { "REQUIRED,json_name=\"__json_request_body\"]")); } +TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactString) { + auto constexpr kFieldJson = R"""( +{ + "type": "string" +} +)"""; + auto json = nlohmann::json::parse(kFieldJson, nullptr, false); + ASSERT_TRUE(json.is_object()); + EXPECT_THAT( + DiscoveryTypeVertex::FormatFieldOptions("raw_key", "rawKey", json), + Eq(" [debug_redact = true,json_name=\"rawKey\"]")); +} + +TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactBytes) { + auto constexpr kFieldJson = R"""( +{ + "type": "bytes" +} +)"""; + auto json = nlohmann::json::parse(kFieldJson, nullptr, false); + ASSERT_TRUE(json.is_object()); + EXPECT_THAT( + DiscoveryTypeVertex::FormatFieldOptions("raw_key", "rawKey", json), + Eq(" [debug_redact = true,json_name=\"rawKey\"]")); +} + +TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactArrayString) { + auto constexpr kFieldJson = R"""( +{ + "type": "array", + "items": { + "type": "string" + } +} +)"""; + auto json = nlohmann::json::parse(kFieldJson, nullptr, false); + ASSERT_TRUE(json.is_object()); + EXPECT_THAT( + DiscoveryTypeVertex::FormatFieldOptions("keys", "keys", json), + Eq(" [debug_redact = true,json_name=\"keys\"]")); +} + +TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactArrayBytes) { + auto constexpr kFieldJson = R"""( +{ + "type": "array", + "items": { + "type": "bytes" + } +} +)"""; + auto json = nlohmann::json::parse(kFieldJson, nullptr, false); + ASSERT_TRUE(json.is_object()); + EXPECT_THAT( + DiscoveryTypeVertex::FormatFieldOptions("keys", "keys", json), + Eq(" [debug_redact = true,json_name=\"keys\"]")); +} + +TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactNonStringNotRedacted) { + auto constexpr kFieldJson = R"""( +{ + "type": "integer" +} +)"""; + auto json = nlohmann::json::parse(kFieldJson, nullptr, false); + ASSERT_TRUE(json.is_object()); + EXPECT_THAT( + DiscoveryTypeVertex::FormatFieldOptions("key_id", "keyId", json), + Eq(" [json_name=\"keyId\"]")); +} + +TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactRequired) { + auto constexpr kFieldJson = R"""( +{ + "type": "string", + "required": true +} +)"""; + auto json = nlohmann::json::parse(kFieldJson, nullptr, false); + ASSERT_TRUE(json.is_object()); + EXPECT_THAT( + DiscoveryTypeVertex::FormatFieldOptions("raw_key", "rawKey", json), + Eq(" [(google.api.field_behavior) = " + "REQUIRED,debug_redact = true,json_name=\"rawKey\"]")); +} + struct DetermineTypesSuccess { std::string name; std::string json; @@ -1312,6 +1398,48 @@ message TestSchema { "optional string to: optional double"))); } +TEST_F(DiscoveryTypeVertexDescriptorTest, + JsonToProtobufCustomerEncryptionKey) { + auto constexpr kSchemaJson = R"""( +{ + "id": "CustomerEncryptionKey", + "properties": { + "kmsKeyName": { + "type": "string" + }, + "rawKey": { + "type": "string" + }, + "rsaEncryptedKey": { + "type": "string" + }, + "sha256": { + "type": "string" + } + } +} +)"""; + + auto constexpr kExpectedProto = R"""(message CustomerEncryptionKey { + optional string kms_key_name = 1 [debug_redact = true,json_name="kmsKeyName"]; + + optional string raw_key = 2 [debug_redact = true,json_name="rawKey"]; + + optional string rsa_encrypted_key = 3 [debug_redact = true,json_name="rsaEncryptedKey"]; + + optional string sha256 = 4 [json_name="sha256"]; +} +)"""; + + auto json = nlohmann::json::parse(kSchemaJson, nullptr, false); + ASSERT_TRUE(json.is_object()); + DiscoveryTypeVertex t("CustomerEncryptionKey", "test.package", json, &pool()); + std::map types; + auto result = t.JsonToProtobufMessage(types, "test.package"); + ASSERT_THAT(result, ::google::cloud::testing_util::IsOk()); + EXPECT_THAT(*result, Eq(kExpectedProto)); +} + } // namespace } // namespace generator_internal } // namespace cloud diff --git a/google/cloud/internal/debug_string_protobuf.cc b/google/cloud/internal/debug_string_protobuf.cc index e652d2458bbd5..15890b4f6933b 100644 --- a/google/cloud/internal/debug_string_protobuf.cc +++ b/google/cloud/internal/debug_string_protobuf.cc @@ -68,6 +68,7 @@ std::string DebugString(google::protobuf::Message const& m, TracingOptions const& options) { std::string str; google::protobuf::TextFormat::Printer p; + p.SetRedactDebugString(true); p.SetSingleLineMode(options.single_line_mode()); if (!options.single_line_mode()) p.SetInitialIndentLevel(1); p.SetUseShortRepeatedPrimitives(options.use_short_repeated_primitives()); diff --git a/google/cloud/internal/debug_string_protobuf_test.cc b/google/cloud/internal/debug_string_protobuf_test.cc index bb6bfed365c57..9d38a046f0a33 100644 --- a/google/cloud/internal/debug_string_protobuf_test.cc +++ b/google/cloud/internal/debug_string_protobuf_test.cc @@ -18,6 +18,9 @@ #include "google/iam/v1/policy.pb.h" #include "google/protobuf/duration.pb.h" #include "google/protobuf/timestamp.pb.h" +#include +#include +#include #include #include @@ -51,6 +54,7 @@ TEST(LogWrapperHelpers, DefaultOptions) { // clang-format off std::string const text = R"pb(google.iam.v1.Policy { )pb" + R"pb(goo.gle/debugproto )pb" R"pb(bindings { )pb" R"pb(role: "roles/viewer" )pb" R"pb(members: "user:user1@example.com" )pb" @@ -72,6 +76,7 @@ TEST(LogWrapperHelpers, MultiLine) { tracing_options.SetOptions("single_line_mode=off"); // clang-format off std::string const text = R"pb(google.iam.v1.Policy { + goo.gle/debugproto bindings { role: "roles/viewer" members: "user:user1@example.com" @@ -94,6 +99,7 @@ TEST(LogWrapperHelpers, Truncate) { // clang-format off std::string const text = R"pb(google.iam.v1.Policy { )pb" + R"pb(goo.gle/debugproto )pb" R"pb(bindings { )pb" R"pb(role: "roles/vi......" )pb" R"pb(members: "user:use......" )pb" @@ -115,7 +121,7 @@ TEST(LogWrapperHelpers, Duration) { duration.set_seconds((11 * 60 + 22) * 60 + 33); duration.set_nanos(123456789); std::string const expected = - R"(google.protobuf.Duration { "11h22m33.123456789s" })"; + R"(google.protobuf.Duration { goo.gle/debugproto "11h22m33.123456789s" })"; EXPECT_EQ(expected, DebugString(duration, TracingOptions{}.SetOptions( "single_line_mode=on"))); } @@ -125,12 +131,55 @@ TEST(LogWrapperHelpers, Timestamp) { timestamp.set_seconds(1658470436); timestamp.set_nanos(123456789); std::string const expected = R"(google.protobuf.Timestamp { + goo.gle/debugproto "2022-07-22T06:13:56.123456789Z" })"; EXPECT_EQ(expected, DebugString(timestamp, TracingOptions{}.SetOptions( "single_line_mode=off"))); } +TEST(LogWrapperHelpers, RedactedField) { + google::protobuf::FileDescriptorProto file_proto; + file_proto.set_name("test_redact.proto"); + file_proto.set_syntax("proto3"); + google::protobuf::DescriptorProto* message_proto = + file_proto.add_message_type(); + message_proto->set_name("RedactedMessage"); + + google::protobuf::FieldDescriptorProto* unredacted_field = + message_proto->add_field(); + unredacted_field->set_name("public_field"); + unredacted_field->set_number(1); + unredacted_field->set_type(google::protobuf::FieldDescriptorProto::TYPE_STRING); + + google::protobuf::FieldDescriptorProto* redacted_field = + message_proto->add_field(); + redacted_field->set_name("secret_key"); + redacted_field->set_number(2); + redacted_field->set_type(google::protobuf::FieldDescriptorProto::TYPE_STRING); + redacted_field->mutable_options()->set_debug_redact(true); + + google::protobuf::DescriptorPool pool; + google::protobuf::FileDescriptor const* file_desc = + pool.BuildFile(file_proto); + ASSERT_THAT(file_desc, ::testing::NotNull()); + google::protobuf::Descriptor const* msg_desc = + file_desc->FindMessageTypeByName("RedactedMessage"); + ASSERT_THAT(msg_desc, ::testing::NotNull()); + + google::protobuf::DynamicMessageFactory factory; + std::unique_ptr message( + factory.GetPrototype(msg_desc)->New()); + google::protobuf::Reflection const* reflection = message->GetReflection(); + reflection->SetString(message.get(), msg_desc->field(0), "public_value"); + reflection->SetString(message.get(), msg_desc->field(1), "secret_value"); + + TracingOptions tracing_options; + std::string const actual = DebugString(*message, tracing_options); + EXPECT_THAT(actual, ::testing::HasSubstr("public_value")); + EXPECT_THAT(actual, ::testing::Not(::testing::HasSubstr("secret_value"))); +} + } // namespace } // namespace internal GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END From 1e723fc6294c2f9fb46773a9e9e78e44e23b5625 Mon Sep 17 00:00:00 2001 From: Colin Moy Date: Thu, 27 Aug 2026 20:11:59 +0000 Subject: [PATCH 3/5] fix(generator): address review feedback on key word-boundary matching and JSON type safety --- generator/internal/discovery_to_proto.cc | 10 +++-- generator/internal/discovery_type_vertex.cc | 42 +++++++++++++------ .../internal/discovery_type_vertex_test.cc | 30 +++++++++++-- 3 files changed, 61 insertions(+), 21 deletions(-) diff --git a/generator/internal/discovery_to_proto.cc b/generator/internal/discovery_to_proto.cc index 0baf70a01b4c3..6dab429dece48 100644 --- a/generator/internal/discovery_to_proto.cc +++ b/generator/internal/discovery_to_proto.cc @@ -360,7 +360,7 @@ std::set FindAllTypesToImport(nlohmann::json const& json) { types_to_import.insert((*current)["$ref"]); } - if (current->contains("format")) { + if (current->contains("format") && (*current)["format"].is_string()) { std::string const format = (*current)["format"]; if (absl::StartsWith(format, "google.protobuf.")) { types_to_import.insert(format); @@ -383,8 +383,9 @@ std::set FindAllTypesToImport(nlohmann::json const& json) { if (IsDiscoveryArrayType(*current)) { auto const& items = (*current)["items"]; - if (items.contains("type") && items["type"] == "object" && - items.contains("additionalProperties") && + if (items.is_object() && items.contains("type") && + items["type"] == "object" && items.contains("additionalProperties") && + items["additionalProperties"].is_object() && items["additionalProperties"].value("type", "") == "any" && !items.contains("format") && !items["additionalProperties"].contains("format")) { @@ -396,7 +397,8 @@ std::set FindAllTypesToImport(nlohmann::json const& json) { if (IsDiscoveryMapType(*current)) { auto const& additional_properties = (*current)["additionalProperties"]; - if (additional_properties.contains("type") && + if (additional_properties.is_object() && + additional_properties.contains("type") && additional_properties["type"] == "any" && !additional_properties.contains("format") && !current->contains("format")) { diff --git a/generator/internal/discovery_type_vertex.cc b/generator/internal/discovery_type_vertex.cc index f6666f69e3cae..36f1e44080464 100644 --- a/generator/internal/discovery_type_vertex.cc +++ b/generator/internal/discovery_type_vertex.cc @@ -48,13 +48,24 @@ std::optional CheckForScalarType(nlohmann::json const& j) { bool IsStringOrBytes(nlohmann::json const& field_json) { std::string const type = field_json.value("type", ""); if (type == "string" || type == "bytes") return true; - if (type == "array" && field_json.contains("items")) { + if (type == "array" && field_json.contains("items") && + field_json["items"].is_object()) { std::string const item_type = field_json["items"].value("type", ""); if (item_type == "string" || item_type == "bytes") return true; } return false; } +bool ContainsKeyWord(std::string_view s) { + for (std::size_t pos = s.find("key"); pos != std::string_view::npos; + pos = s.find("key", pos + 1)) { + bool const prefix_ok = (pos == 0 || s[pos - 1] == '_'); + bool const suffix_ok = (pos + 3 == s.size() || s[pos + 3] == '_'); + if (prefix_ok && suffix_ok) return true; + } + return false; +} + } // namespace DiscoveryTypeVertex::DiscoveryTypeVertex( @@ -122,7 +133,7 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, } if (type == "any") { - if (v.contains("format")) { + if (v.contains("format") && v["format"].is_string()) { type = v["format"]; } else { type = "google.protobuf.Value"; @@ -163,9 +174,11 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, properties_for_synthesis = &additional_properties; is_message = true; } else if (map_type == "any") { - if (additional_properties.contains("format")) { + if (additional_properties.is_object() && + additional_properties.contains("format") && + additional_properties["format"].is_string()) { map_type = additional_properties["format"]; - } else if (v.contains("format")) { + } else if (v.contains("format") && v["format"].is_string()) { map_type = v["format"]; } else { map_type = "google.protobuf.Struct"; @@ -206,21 +219,26 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, if (scalar_type) { type = *scalar_type; } else if (type == "any") { - if (items.contains("format")) { + if (items.is_object() && items.contains("format") && + items["format"].is_string()) { type = items["format"]; } else { type = "google.protobuf.Value"; } return TypeInfo{type, compare_package_name, nullptr, false, false}; - } else if (type == "object" && items.contains("properties")) { + } else if (type == "object" && items.is_object() && + items.contains("properties")) { // Synthesize a nested type for this array. type = CapitalizeFirstLetter(field_name + "Item"); return TypeInfo{type, compare_package_name, &items, false, true}; - } else if (type == "object" && items.contains("additionalProperties") && - (items["additionalProperties"]).value("type", "") == "any") { - if (items.contains("format")) { + } else if (type == "object" && items.is_object() && + items.contains("additionalProperties") && + items["additionalProperties"].is_object() && + items["additionalProperties"].value("type", "") == "any") { + if (items.contains("format") && items["format"].is_string()) { type = items["format"]; - } else if (items["additionalProperties"].contains("format")) { + } else if (items["additionalProperties"].contains("format") && + items["additionalProperties"]["format"].is_string()) { type = items["additionalProperties"]["format"]; } else { type = "google.protobuf.Struct"; @@ -507,9 +525,7 @@ std::string DiscoveryTypeVertex::FormatFieldOptions( absl::StrCat("\"", field_name, "\"")); } - if (IsStringOrBytes(field_json) && - (absl::StrContains(field_name, "key") || - absl::StrContains(absl::AsciiStrToLower(json_field_name), "key"))) { + if (IsStringOrBytes(field_json) && ContainsKeyWord(field_name)) { field_options.emplace_back("debug_redact", "true"); } diff --git a/generator/internal/discovery_type_vertex_test.cc b/generator/internal/discovery_type_vertex_test.cc index 62fcb454dcfae..5f583c9e29152 100644 --- a/generator/internal/discovery_type_vertex_test.cc +++ b/generator/internal/discovery_type_vertex_test.cc @@ -220,8 +220,8 @@ TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactArrayString) { auto json = nlohmann::json::parse(kFieldJson, nullptr, false); ASSERT_TRUE(json.is_object()); EXPECT_THAT( - DiscoveryTypeVertex::FormatFieldOptions("keys", "keys", json), - Eq(" [debug_redact = true,json_name=\"keys\"]")); + DiscoveryTypeVertex::FormatFieldOptions("raw_key", "rawKey", json), + Eq(" [debug_redact = true,json_name=\"rawKey\"]")); } TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactArrayBytes) { @@ -236,8 +236,30 @@ TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactArrayBytes) { auto json = nlohmann::json::parse(kFieldJson, nullptr, false); ASSERT_TRUE(json.is_object()); EXPECT_THAT( - DiscoveryTypeVertex::FormatFieldOptions("keys", "keys", json), - Eq(" [debug_redact = true,json_name=\"keys\"]")); + DiscoveryTypeVertex::FormatFieldOptions("raw_key", "rawKey", json), + Eq(" [debug_redact = true,json_name=\"rawKey\"]")); +} + +TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactNonMatches) { + auto constexpr kFieldJson = R"""( +{ + "type": "string" +} +)"""; + auto json = nlohmann::json::parse(kFieldJson, nullptr, false); + ASSERT_TRUE(json.is_object()); + EXPECT_THAT( + DiscoveryTypeVertex::FormatFieldOptions("monkey", "monkey", json), + Eq(" [json_name=\"monkey\"]")); + EXPECT_THAT( + DiscoveryTypeVertex::FormatFieldOptions("keyboard", "keyboard", json), + Eq(" [json_name=\"keyboard\"]")); + EXPECT_THAT( + DiscoveryTypeVertex::FormatFieldOptions("hockey", "hockey", json), + Eq(" [json_name=\"hockey\"]")); + EXPECT_THAT( + DiscoveryTypeVertex::FormatFieldOptions("keypad", "keypad", json), + Eq(" [json_name=\"keypad\"]")); } TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactNonStringNotRedacted) { From fb425870d1b761d6275e5062756155ceae78dc08 Mon Sep 17 00:00:00 2001 From: Colin Moy Date: Thu, 27 Aug 2026 22:37:54 +0000 Subject: [PATCH 4/5] fix(generator): address maintainer review feedback and fix compilation across protobuf versions --- generator/internal/discovery_to_proto.cc | 10 +++--- generator/internal/discovery_type_vertex.cc | 35 +++++++------------ .../cloud/internal/debug_string_protobuf.cc | 11 +++++- .../internal/debug_string_protobuf_test.cc | 10 +++--- 4 files changed, 31 insertions(+), 35 deletions(-) diff --git a/generator/internal/discovery_to_proto.cc b/generator/internal/discovery_to_proto.cc index 6dab429dece48..0baf70a01b4c3 100644 --- a/generator/internal/discovery_to_proto.cc +++ b/generator/internal/discovery_to_proto.cc @@ -360,7 +360,7 @@ std::set FindAllTypesToImport(nlohmann::json const& json) { types_to_import.insert((*current)["$ref"]); } - if (current->contains("format") && (*current)["format"].is_string()) { + if (current->contains("format")) { std::string const format = (*current)["format"]; if (absl::StartsWith(format, "google.protobuf.")) { types_to_import.insert(format); @@ -383,9 +383,8 @@ std::set FindAllTypesToImport(nlohmann::json const& json) { if (IsDiscoveryArrayType(*current)) { auto const& items = (*current)["items"]; - if (items.is_object() && items.contains("type") && - items["type"] == "object" && items.contains("additionalProperties") && - items["additionalProperties"].is_object() && + if (items.contains("type") && items["type"] == "object" && + items.contains("additionalProperties") && items["additionalProperties"].value("type", "") == "any" && !items.contains("format") && !items["additionalProperties"].contains("format")) { @@ -397,8 +396,7 @@ std::set FindAllTypesToImport(nlohmann::json const& json) { if (IsDiscoveryMapType(*current)) { auto const& additional_properties = (*current)["additionalProperties"]; - if (additional_properties.is_object() && - additional_properties.contains("type") && + if (additional_properties.contains("type") && additional_properties["type"] == "any" && !additional_properties.contains("format") && !current->contains("format")) { diff --git a/generator/internal/discovery_type_vertex.cc b/generator/internal/discovery_type_vertex.cc index 36f1e44080464..85be70bde7e65 100644 --- a/generator/internal/discovery_type_vertex.cc +++ b/generator/internal/discovery_type_vertex.cc @@ -48,8 +48,7 @@ std::optional CheckForScalarType(nlohmann::json const& j) { bool IsStringOrBytes(nlohmann::json const& field_json) { std::string const type = field_json.value("type", ""); if (type == "string" || type == "bytes") return true; - if (type == "array" && field_json.contains("items") && - field_json["items"].is_object()) { + if (type == "array" && field_json.contains("items")) { std::string const item_type = field_json["items"].value("type", ""); if (item_type == "string" || item_type == "bytes") return true; } @@ -57,11 +56,8 @@ bool IsStringOrBytes(nlohmann::json const& field_json) { } bool ContainsKeyWord(std::string_view s) { - for (std::size_t pos = s.find("key"); pos != std::string_view::npos; - pos = s.find("key", pos + 1)) { - bool const prefix_ok = (pos == 0 || s[pos - 1] == '_'); - bool const suffix_ok = (pos + 3 == s.size() || s[pos + 3] == '_'); - if (prefix_ok && suffix_ok) return true; + for (auto const& token : absl::StrSplit(s, '_')) { + if (token == "key") return true; } return false; } @@ -133,7 +129,7 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, } if (type == "any") { - if (v.contains("format") && v["format"].is_string()) { + if (v.contains("format")) { type = v["format"]; } else { type = "google.protobuf.Value"; @@ -174,11 +170,9 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, properties_for_synthesis = &additional_properties; is_message = true; } else if (map_type == "any") { - if (additional_properties.is_object() && - additional_properties.contains("format") && - additional_properties["format"].is_string()) { + if (additional_properties.contains("format")) { map_type = additional_properties["format"]; - } else if (v.contains("format") && v["format"].is_string()) { + } else if (v.contains("format")) { map_type = v["format"]; } else { map_type = "google.protobuf.Struct"; @@ -219,26 +213,21 @@ DiscoveryTypeVertex::DetermineTypeAndSynthesis(nlohmann::json const& v, if (scalar_type) { type = *scalar_type; } else if (type == "any") { - if (items.is_object() && items.contains("format") && - items["format"].is_string()) { + if (items.contains("format")) { type = items["format"]; } else { type = "google.protobuf.Value"; } return TypeInfo{type, compare_package_name, nullptr, false, false}; - } else if (type == "object" && items.is_object() && - items.contains("properties")) { + } else if (type == "object" && items.contains("properties")) { // Synthesize a nested type for this array. type = CapitalizeFirstLetter(field_name + "Item"); return TypeInfo{type, compare_package_name, &items, false, true}; - } else if (type == "object" && items.is_object() && - items.contains("additionalProperties") && - items["additionalProperties"].is_object() && - items["additionalProperties"].value("type", "") == "any") { - if (items.contains("format") && items["format"].is_string()) { + } else if (type == "object" && items.contains("additionalProperties") && + (items["additionalProperties"]).value("type", "") == "any") { + if (items.contains("format")) { type = items["format"]; - } else if (items["additionalProperties"].contains("format") && - items["additionalProperties"]["format"].is_string()) { + } else if (items["additionalProperties"].contains("format")) { type = items["additionalProperties"]["format"]; } else { type = "google.protobuf.Struct"; diff --git a/google/cloud/internal/debug_string_protobuf.cc b/google/cloud/internal/debug_string_protobuf.cc index 15890b4f6933b..c65c56df9bcb8 100644 --- a/google/cloud/internal/debug_string_protobuf.cc +++ b/google/cloud/internal/debug_string_protobuf.cc @@ -62,13 +62,22 @@ class TimestampMessagePrinter } }; +template +auto SetRedact(Printer& p, int) + -> decltype(p.SetRedactDebugString(true), void()) { + p.SetRedactDebugString(true); +} + +template +void SetRedact(Printer&, ...) {} + } // namespace std::string DebugString(google::protobuf::Message const& m, TracingOptions const& options) { std::string str; google::protobuf::TextFormat::Printer p; - p.SetRedactDebugString(true); + SetRedact(p, 0); p.SetSingleLineMode(options.single_line_mode()); if (!options.single_line_mode()) p.SetInitialIndentLevel(1); p.SetUseShortRepeatedPrimitives(options.use_short_repeated_primitives()); diff --git a/google/cloud/internal/debug_string_protobuf_test.cc b/google/cloud/internal/debug_string_protobuf_test.cc index 9d38a046f0a33..7af21c16c0193 100644 --- a/google/cloud/internal/debug_string_protobuf_test.cc +++ b/google/cloud/internal/debug_string_protobuf_test.cc @@ -54,7 +54,7 @@ TEST(LogWrapperHelpers, DefaultOptions) { // clang-format off std::string const text = R"pb(google.iam.v1.Policy { )pb" - R"pb(goo.gle/debugproto )pb" + R"pb(goo.gle/debugonly )pb" R"pb(bindings { )pb" R"pb(role: "roles/viewer" )pb" R"pb(members: "user:user1@example.com" )pb" @@ -76,7 +76,7 @@ TEST(LogWrapperHelpers, MultiLine) { tracing_options.SetOptions("single_line_mode=off"); // clang-format off std::string const text = R"pb(google.iam.v1.Policy { - goo.gle/debugproto + goo.gle/debugonly bindings { role: "roles/viewer" members: "user:user1@example.com" @@ -99,7 +99,7 @@ TEST(LogWrapperHelpers, Truncate) { // clang-format off std::string const text = R"pb(google.iam.v1.Policy { )pb" - R"pb(goo.gle/debugproto )pb" + R"pb(goo.gle/debugonly )pb" R"pb(bindings { )pb" R"pb(role: "roles/vi......" )pb" R"pb(members: "user:use......" )pb" @@ -121,7 +121,7 @@ TEST(LogWrapperHelpers, Duration) { duration.set_seconds((11 * 60 + 22) * 60 + 33); duration.set_nanos(123456789); std::string const expected = - R"(google.protobuf.Duration { goo.gle/debugproto "11h22m33.123456789s" })"; + R"(google.protobuf.Duration { goo.gle/debugonly "11h22m33.123456789s" })"; EXPECT_EQ(expected, DebugString(duration, TracingOptions{}.SetOptions( "single_line_mode=on"))); } @@ -131,7 +131,7 @@ TEST(LogWrapperHelpers, Timestamp) { timestamp.set_seconds(1658470436); timestamp.set_nanos(123456789); std::string const expected = R"(google.protobuf.Timestamp { - goo.gle/debugproto + goo.gle/debugonly "2022-07-22T06:13:56.123456789Z" })"; EXPECT_EQ(expected, DebugString(timestamp, TracingOptions{}.SetOptions( From 672d0fc3b02f2d61844df0006e170ddcd8e83c75 Mon Sep 17 00:00:00 2001 From: Colin Moy Date: Fri, 28 Aug 2026 19:35:06 +0000 Subject: [PATCH 5/5] fix(generator): clean up protobuf debug markers and format test expectations --- .../internal/discovery_type_vertex_test.cc | 26 ++++++++----------- .../cloud/internal/debug_string_protobuf.cc | 21 +++++++++++++-- .../internal/debug_string_protobuf_test.cc | 9 +++---- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/generator/internal/discovery_type_vertex_test.cc b/generator/internal/discovery_type_vertex_test.cc index 5f583c9e29152..1a03d3ad7f3d1 100644 --- a/generator/internal/discovery_type_vertex_test.cc +++ b/generator/internal/discovery_type_vertex_test.cc @@ -248,21 +248,19 @@ TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactNonMatches) { )"""; auto json = nlohmann::json::parse(kFieldJson, nullptr, false); ASSERT_TRUE(json.is_object()); - EXPECT_THAT( - DiscoveryTypeVertex::FormatFieldOptions("monkey", "monkey", json), - Eq(" [json_name=\"monkey\"]")); + EXPECT_THAT(DiscoveryTypeVertex::FormatFieldOptions("monkey", "monkey", json), + Eq(" [json_name=\"monkey\"]")); EXPECT_THAT( DiscoveryTypeVertex::FormatFieldOptions("keyboard", "keyboard", json), Eq(" [json_name=\"keyboard\"]")); - EXPECT_THAT( - DiscoveryTypeVertex::FormatFieldOptions("hockey", "hockey", json), - Eq(" [json_name=\"hockey\"]")); - EXPECT_THAT( - DiscoveryTypeVertex::FormatFieldOptions("keypad", "keypad", json), - Eq(" [json_name=\"keypad\"]")); + EXPECT_THAT(DiscoveryTypeVertex::FormatFieldOptions("hockey", "hockey", json), + Eq(" [json_name=\"hockey\"]")); + EXPECT_THAT(DiscoveryTypeVertex::FormatFieldOptions("keypad", "keypad", json), + Eq(" [json_name=\"keypad\"]")); } -TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactNonStringNotRedacted) { +TEST(DiscoveryTypeVertexTest, + FormatFieldOptionsDebugRedactNonStringNotRedacted) { auto constexpr kFieldJson = R"""( { "type": "integer" @@ -270,9 +268,8 @@ TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactNonStringNotRedacted) )"""; auto json = nlohmann::json::parse(kFieldJson, nullptr, false); ASSERT_TRUE(json.is_object()); - EXPECT_THAT( - DiscoveryTypeVertex::FormatFieldOptions("key_id", "keyId", json), - Eq(" [json_name=\"keyId\"]")); + EXPECT_THAT(DiscoveryTypeVertex::FormatFieldOptions("key_id", "keyId", json), + Eq(" [json_name=\"keyId\"]")); } TEST(DiscoveryTypeVertexTest, FormatFieldOptionsDebugRedactRequired) { @@ -1420,8 +1417,7 @@ message TestSchema { "optional string to: optional double"))); } -TEST_F(DiscoveryTypeVertexDescriptorTest, - JsonToProtobufCustomerEncryptionKey) { +TEST_F(DiscoveryTypeVertexDescriptorTest, JsonToProtobufCustomerEncryptionKey) { auto constexpr kSchemaJson = R"""( { "id": "CustomerEncryptionKey", diff --git a/google/cloud/internal/debug_string_protobuf.cc b/google/cloud/internal/debug_string_protobuf.cc index c65c56df9bcb8..6a9abb2538650 100644 --- a/google/cloud/internal/debug_string_protobuf.cc +++ b/google/cloud/internal/debug_string_protobuf.cc @@ -63,14 +63,30 @@ class TimestampMessagePrinter }; template -auto SetRedact(Printer& p, int) - -> decltype(p.SetRedactDebugString(true), void()) { +auto SetRedact(Printer& p, int) -> decltype(p.SetRedactDebugString(true), + void()) { p.SetRedactDebugString(true); } template void SetRedact(Printer&, ...) {} +void RemoveSilentMarker(std::string& str) { + auto start = str.find("goo.gle/"); + if (start != std::string::npos) { + auto nl = str.find('\n', start); + if (nl != std::string::npos) { + str.erase(0, nl + 1); + } else { + auto end = str.find_first_of(" \t", start); + if (end != std::string::npos) { + auto next = str.find_first_not_of(" \t", end); + str.erase(0, next == std::string::npos ? str.size() : next); + } + } + } +} + } // namespace std::string DebugString(google::protobuf::Message const& m, @@ -90,6 +106,7 @@ std::string DebugString(google::protobuf::Message const& m, p.RegisterMessagePrinter(google::protobuf::Timestamp::descriptor(), new TimestampMessagePrinter); p.PrintToString(m, &str); + RemoveSilentMarker(str); return absl::StrCat(m.GetTypeName(), " {", (options.single_line_mode() ? " " : "\n"), str, "}"); } diff --git a/google/cloud/internal/debug_string_protobuf_test.cc b/google/cloud/internal/debug_string_protobuf_test.cc index 7af21c16c0193..acbd1d88cd97e 100644 --- a/google/cloud/internal/debug_string_protobuf_test.cc +++ b/google/cloud/internal/debug_string_protobuf_test.cc @@ -54,7 +54,6 @@ TEST(LogWrapperHelpers, DefaultOptions) { // clang-format off std::string const text = R"pb(google.iam.v1.Policy { )pb" - R"pb(goo.gle/debugonly )pb" R"pb(bindings { )pb" R"pb(role: "roles/viewer" )pb" R"pb(members: "user:user1@example.com" )pb" @@ -76,7 +75,6 @@ TEST(LogWrapperHelpers, MultiLine) { tracing_options.SetOptions("single_line_mode=off"); // clang-format off std::string const text = R"pb(google.iam.v1.Policy { - goo.gle/debugonly bindings { role: "roles/viewer" members: "user:user1@example.com" @@ -99,7 +97,6 @@ TEST(LogWrapperHelpers, Truncate) { // clang-format off std::string const text = R"pb(google.iam.v1.Policy { )pb" - R"pb(goo.gle/debugonly )pb" R"pb(bindings { )pb" R"pb(role: "roles/vi......" )pb" R"pb(members: "user:use......" )pb" @@ -121,7 +118,7 @@ TEST(LogWrapperHelpers, Duration) { duration.set_seconds((11 * 60 + 22) * 60 + 33); duration.set_nanos(123456789); std::string const expected = - R"(google.protobuf.Duration { goo.gle/debugonly "11h22m33.123456789s" })"; + R"(google.protobuf.Duration { "11h22m33.123456789s" })"; EXPECT_EQ(expected, DebugString(duration, TracingOptions{}.SetOptions( "single_line_mode=on"))); } @@ -131,7 +128,6 @@ TEST(LogWrapperHelpers, Timestamp) { timestamp.set_seconds(1658470436); timestamp.set_nanos(123456789); std::string const expected = R"(google.protobuf.Timestamp { - goo.gle/debugonly "2022-07-22T06:13:56.123456789Z" })"; EXPECT_EQ(expected, DebugString(timestamp, TracingOptions{}.SetOptions( @@ -150,7 +146,8 @@ TEST(LogWrapperHelpers, RedactedField) { message_proto->add_field(); unredacted_field->set_name("public_field"); unredacted_field->set_number(1); - unredacted_field->set_type(google::protobuf::FieldDescriptorProto::TYPE_STRING); + unredacted_field->set_type( + google::protobuf::FieldDescriptorProto::TYPE_STRING); google::protobuf::FieldDescriptorProto* redacted_field = message_proto->add_field();