minor opt: minor optimization to the orca parser

source/common/orca/orca_parser.cc

@@ -153,25 +153,32 @@ absl::StatusOr<OrcaLoadReport> parseOrcaLoadReportHeaders(const HeaderMap& heade
     const auto header_value = header_bin[0]->value().getStringView();
     RETURN_IF_NOT_OK(tryParseSerializedBinary(header_value, load_report));
   } else if (const auto header = headers.get(endpointLoadMetricsHeader()); !header.empty()) {
-    std::pair<absl::string_view, absl::string_view> split_header =
-        absl::StrSplit(header[0]->value().getStringView(), absl::MaxSplits(' ', 1));
-
-    if (split_header.first == kHeaderFormatPrefixBin) { // Binary protobuf format.
-      RETURN_IF_NOT_OK(tryParseSerializedBinary(split_header.second, load_report));
-    } else if (split_header.first == kHeaderFormatPrefixText) { // Native HTTP format.
-      RETURN_IF_NOT_OK(tryParseNativeHttpEncoded(split_header.second, load_report));
-    } else if (split_header.first == kHeaderFormatPrefixJson) { // JSON format.
+    absl::string_view header_value = header[0]->value().getStringView();
+
+    if (absl::StartsWith(header_value, kHeaderFormatPrefixBin)) {
+      // Binary protobuf format.
+      RETURN_IF_NOT_OK(tryParseSerializedBinary(header_value.substr(kHeaderFormatPrefixBin.size()),
+                                                load_report));
+    } else if (absl::StartsWith(header_value, kHeaderFormatPrefixText)) {
+      // Native HTTP format.
+      RETURN_IF_NOT_OK(tryParseNativeHttpEncoded(
+          header_value.substr(kHeaderFormatPrefixText.size()), load_report));
+    } else if (absl::StartsWith(header_value, kHeaderFormatPrefixJson)) {
+      // JSON format.
 #if defined(ENVOY_ENABLE_FULL_PROTOS) && defined(ENVOY_ENABLE_YAML)
-      const std::string json_string = std::string(split_header.second);
       bool has_unknown_field = false;
-      RETURN_IF_ERROR(
-          Envoy::MessageUtil::loadFromJsonNoThrow(json_string, load_report, has_unknown_field));
+      RETURN_IF_ERROR(Envoy::MessageUtil::loadFromJsonNoThrow(
+          header_value.substr(kHeaderFormatPrefixJson.size()), load_report, has_unknown_field));
 #else
       IS_ENVOY_BUG("JSON formatted ORCA header support not implemented for this build");
 #endif // !ENVOY_ENABLE_FULL_PROTOS || !ENVOY_ENABLE_YAML
     } else {
-      return absl::InvalidArgumentError(
-          fmt::format("unsupported ORCA header format: {}", split_header.first));
+      // Unknown format. Get the first 5 characters or the prefix before the first space to
+      // generate the error message.
+      absl::string_view prefix = header_value.substr(0, std::min<size_t>(5, header_value.size()));
+      prefix = prefix.substr(0, prefix.find_first_of(' '));
+
+      return absl::InvalidArgumentError(fmt::format("unsupported ORCA header format: {}", prefix));
     }
   } else {
     return absl::NotFoundError("no ORCA data sent from the backend");

source/common/orca/orca_parser.h

@@ -12,9 +12,9 @@ namespace Orca {
 static constexpr absl::string_view kEndpointLoadMetricsHeader = "endpoint-load-metrics";
 static constexpr absl::string_view kEndpointLoadMetricsHeaderBin = "endpoint-load-metrics-bin";
 // Prefix used to determine format expected in kEndpointLoadMetricsHeader.
-static constexpr absl::string_view kHeaderFormatPrefixBin = "BIN";
-static constexpr absl::string_view kHeaderFormatPrefixJson = "JSON";
-static constexpr absl::string_view kHeaderFormatPrefixText = "TEXT";
+static constexpr absl::string_view kHeaderFormatPrefixBin = "BIN ";
+static constexpr absl::string_view kHeaderFormatPrefixJson = "JSON ";
+static constexpr absl::string_view kHeaderFormatPrefixText = "TEXT ";
 // The following fields are the names of the metrics tracked in the ORCA load
 // report proto.
 static constexpr absl::string_view kApplicationUtilizationField = "application_utilization";

source/common/protobuf/utility.h

@@ -255,7 +255,7 @@ class MessageUtil {
   static std::size_t hash(const Protobuf::Message& message);
 
 #ifdef ENVOY_ENABLE_YAML
-  static void loadFromJson(const std::string& json, Protobuf::Message& message,
+  static void loadFromJson(absl::string_view json, Protobuf::Message& message,
                            ProtobufMessage::ValidationVisitor& validation_visitor);
   /**
    * Return ok only when strict conversion(don't ignore unknown field) succeeds.
@@ -264,9 +264,9 @@ class MessageUtil {
    * Return error status for relaxed conversion and set has_unknown_field to false if relaxed
    * conversion(ignore unknown field) fails.
    */
-  static absl::Status loadFromJsonNoThrow(const std::string& json, Protobuf::Message& message,
+  static absl::Status loadFromJsonNoThrow(absl::string_view json, Protobuf::Message& message,
                                           bool& has_unknown_fileld);
-  static void loadFromJson(const std::string& json, ProtobufWkt::Struct& message);
+  static void loadFromJson(absl::string_view json, ProtobufWkt::Struct& message);
   static void loadFromYaml(const std::string& yaml, Protobuf::Message& message,
                            ProtobufMessage::ValidationVisitor& validation_visitor);
 #endif

source/common/protobuf/yaml_utility.cc

@@ -115,7 +115,7 @@ void jsonConvertInternal(const Protobuf::Message& source,
 
 } // namespace
 
-void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& message,
+void MessageUtil::loadFromJson(absl::string_view json, Protobuf::Message& message,
                                ProtobufMessage::ValidationVisitor& validation_visitor) {
   bool has_unknown_field;
   auto load_status = loadFromJsonNoThrow(json, message, has_unknown_field);
@@ -124,15 +124,16 @@ void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& messa
   }
   if (has_unknown_field) {
     // If the parsing failure is caused by the unknown fields.
-    THROW_IF_NOT_OK(validation_visitor.onUnknownField("type " + message.GetTypeName() + " reason " +
-                                                      load_status.ToString()));
+    THROW_IF_NOT_OK(validation_visitor.onUnknownField(
+        fmt::format("type {} reason {}", message.GetTypeName(), load_status.ToString())));
   } else {
     // If the error has nothing to do with unknown field.
-    throw EnvoyException("Unable to parse JSON as proto (" + load_status.ToString() + "): " + json);
+    throw EnvoyException(
+        fmt::format("Unable to parse JSON as proto ({}): {}", load_status.ToString(), json));
   }
 }
 
-absl::Status MessageUtil::loadFromJsonNoThrow(const std::string& json, Protobuf::Message& message,
+absl::Status MessageUtil::loadFromJsonNoThrow(absl::string_view json, Protobuf::Message& message,
                                               bool& has_unknown_fileld) {
   has_unknown_fileld = false;
   Protobuf::util::JsonParseOptions options;
@@ -163,7 +164,7 @@ absl::Status MessageUtil::loadFromJsonNoThrow(const std::string& json, Protobuf:
   return relaxed_status;
 }
 
-void MessageUtil::loadFromJson(const std::string& json, ProtobufWkt::Struct& message) {
+void MessageUtil::loadFromJson(absl::string_view json, ProtobufWkt::Struct& message) {
   // No need to validate if converting to a Struct, since there are no unknown
   // fields possible.
   loadFromJson(json, message, ProtobufMessage::getNullValidationVisitor());

test/common/orca/orca_parser_test.cc

@@ -14,18 +14,6 @@ namespace Envoy {
 namespace Orca {
 namespace {
 
-const std::string formattedHeaderPrefixText() {
-  CONSTRUCT_ON_FIRST_USE(std::string, absl::StrCat(kHeaderFormatPrefixText, " "));
-}
-
-const std::string formattedHeaderPrefixJson() {
-  CONSTRUCT_ON_FIRST_USE(std::string, absl::StrCat(kHeaderFormatPrefixJson, " "));
-}
-
-const std::string formattedHeaderPrefixBin() {
-  CONSTRUCT_ON_FIRST_USE(std::string, absl::StrCat(kHeaderFormatPrefixBin, " "));
-}
-
 // Returns an example OrcaLoadReport proto with all fields populated.
 static xds::data::orca::v3::OrcaLoadReport exampleOrcaLoadReport() {
   xds::data::orca::v3::OrcaLoadReport orca_load_report;
@@ -65,6 +53,14 @@ TEST(OrcaParserUtilTest, InvalidOrcaHeaderPrefix) {
       StatusHelpers::HasStatus(absl::InvalidArgumentError("unsupported ORCA header format: BAD")));
 }
 
+TEST(OrcaParserUtilTest, InvalidOrcaHeaderPrefixWithLargePrefix) {
+  Http::TestRequestHeaderMapImpl headers{
+      {std::string(kEndpointLoadMetricsHeader), "BADBAD random-value"}};
+  EXPECT_THAT(parseOrcaLoadReportHeaders(headers),
+              StatusHelpers::HasStatus(
+                  absl::InvalidArgumentError("unsupported ORCA header format: BADBA")));
+}
+
 TEST(OrcaParserUtilTest, EmptyOrcaHeader) {
   Http::TestRequestHeaderMapImpl headers{{std::string(kEndpointLoadMetricsHeader), ""}};
   EXPECT_THAT(
@@ -75,7 +71,7 @@ TEST(OrcaParserUtilTest, EmptyOrcaHeader) {
 TEST(OrcaParserUtilTest, NativeHttpEncodedHeader) {
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixText(),
+       absl::StrCat(kHeaderFormatPrefixText,
                     "cpu_utilization:0.7,application_utilization:0.8,mem_utilization:0.9,"
                     "rps_fractional:1000,eps:2,"
                     "named_metrics.foo:123,named_metrics.bar:0.2")}};
@@ -86,28 +82,26 @@ TEST(OrcaParserUtilTest, NativeHttpEncodedHeader) {
 TEST(OrcaParserUtilTest, NativeHttpEncodedHeaderIncorrectFieldType) {
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixText(), "cpu_utilization:\"0.7\"")}};
+       absl::StrCat(kHeaderFormatPrefixText, "cpu_utilization:\"0.7\"")}};
   EXPECT_THAT(parseOrcaLoadReportHeaders(headers),
               StatusHelpers::HasStatus(
                   absl::InvalidArgumentError("unable to parse custom backend load metric "
                                              "value(cpu_utilization): \"0.7\"")));
 }
 
 TEST(OrcaParserUtilTest, NativeHttpEncodedHeaderNanMetricValue) {
-  Http::TestRequestHeaderMapImpl headers{
-      {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixText(),
-                    "cpu_utilization:", std::numeric_limits<double>::quiet_NaN())}};
+  Http::TestRequestHeaderMapImpl headers{{std::string(kEndpointLoadMetricsHeader),
+                                          absl::StrCat(kHeaderFormatPrefixText, "cpu_utilization:",
+                                                       std::numeric_limits<double>::quiet_NaN())}};
   EXPECT_THAT(parseOrcaLoadReportHeaders(headers),
               StatusHelpers::HasStatus(absl::InvalidArgumentError(
                   "custom backend load metric value(cpu_utilization) cannot be NaN.")));
 }
 
 TEST(OrcaParserUtilTest, NativeHttpEncodedHeaderInfinityMetricValue) {
-  Http::TestRequestHeaderMapImpl headers{
-      {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixText(),
-                    "cpu_utilization:", std::numeric_limits<double>::infinity())}};
+  Http::TestRequestHeaderMapImpl headers{{std::string(kEndpointLoadMetricsHeader),
+                                          absl::StrCat(kHeaderFormatPrefixText, "cpu_utilization:",
+                                                       std::numeric_limits<double>::infinity())}};
   EXPECT_THAT(parseOrcaLoadReportHeaders(headers),
               StatusHelpers::HasStatus(absl::InvalidArgumentError(
                   "custom backend load metric value(cpu_utilization) cannot be "
@@ -117,7 +111,7 @@ TEST(OrcaParserUtilTest, NativeHttpEncodedHeaderInfinityMetricValue) {
 TEST(OrcaParserUtilTest, NativeHttpEncodedHeaderContainsDuplicateMetric) {
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixText(), "cpu_utilization:0.7,cpu_utilization:0.8")}};
+       absl::StrCat(kHeaderFormatPrefixText, "cpu_utilization:0.7,cpu_utilization:0.8")}};
   EXPECT_THAT(parseOrcaLoadReportHeaders(headers),
               StatusHelpers::HasStatus(absl::AlreadyExistsError(absl::StrCat(
                   kEndpointLoadMetricsHeader, " contains duplicate metric: cpu_utilization"))));
@@ -126,7 +120,7 @@ TEST(OrcaParserUtilTest, NativeHttpEncodedHeaderContainsDuplicateMetric) {
 TEST(OrcaParserUtilTest, NativeHttpEncodedHeaderUnsupportedMetric) {
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixText(), "cpu_utilization:0.7,unsupported_metric:0.8")}};
+       absl::StrCat(kHeaderFormatPrefixText, "cpu_utilization:0.7,unsupported_metric:0.8")}};
   EXPECT_THAT(parseOrcaLoadReportHeaders(headers),
               StatusHelpers::HasStatus(
                   absl::InvalidArgumentError("unsupported metric name: unsupported_metric")));
@@ -136,7 +130,7 @@ TEST(OrcaParserUtilTest, NativeHttpEncodedHeaderContainsDuplicateNamedMetric) {
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
        absl::StrCat(
-           formattedHeaderPrefixText(),
+           kHeaderFormatPrefixText,
            "named_metrics.foo:123,named_metrics.duplicate:123,named_metrics.duplicate:0.2")}};
   EXPECT_THAT(
       parseOrcaLoadReportHeaders(headers),
@@ -147,15 +141,15 @@ TEST(OrcaParserUtilTest, NativeHttpEncodedHeaderContainsDuplicateNamedMetric) {
 TEST(OrcaParserUtilTest, NativeHttpEncodedHeaderContainsEmptyNamedMetricKey) {
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixText(), "named_metrics.:123")}};
+       absl::StrCat(kHeaderFormatPrefixText, "named_metrics.:123")}};
   EXPECT_THAT(parseOrcaLoadReportHeaders(headers),
               StatusHelpers::HasStatus(absl::InvalidArgumentError("named metric key is empty.")));
 }
 
 TEST(OrcaParserUtilTest, InvalidNativeHttpEncodedHeader) {
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixText(), "not-a-list-of-key-value-pairs")}};
+       absl::StrCat(kHeaderFormatPrefixText, "not-a-list-of-key-value-pairs")}};
   EXPECT_THAT(parseOrcaLoadReportHeaders(headers),
               StatusHelpers::HasStatus(
                   absl::InvalidArgumentError("metric values cannot be empty strings")));
@@ -164,7 +158,7 @@ TEST(OrcaParserUtilTest, InvalidNativeHttpEncodedHeader) {
 TEST(OrcaParserUtilTest, JsonHeader) {
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixJson(),
+       absl::StrCat(kHeaderFormatPrefixJson,
                     "{\"cpu_utilization\": 0.7, \"application_utilization\": 0.8, "
                     "\"mem_utilization\": 0.9, \"rps_fractional\": 1000, \"eps\": 2, "
                     "\"named_metrics\": {\"foo\": 123,\"bar\": 0.2}}")}};
@@ -175,7 +169,7 @@ TEST(OrcaParserUtilTest, JsonHeader) {
 TEST(OrcaParserUtilTest, InvalidJsonHeader) {
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixJson(), "JSON not-a-valid-json-string")}};
+       absl::StrCat(kHeaderFormatPrefixJson, "JSON not-a-valid-json-string")}};
   EXPECT_THAT(parseOrcaLoadReportHeaders(headers),
               StatusHelpers::HasStatus(absl::StatusCode::kInvalidArgument,
                                        testing::HasSubstr("invalid JSON")));
@@ -184,7 +178,7 @@ TEST(OrcaParserUtilTest, InvalidJsonHeader) {
 TEST(OrcaParserUtilTest, JsonHeaderUnknownField) {
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixJson(),
+       absl::StrCat(kHeaderFormatPrefixJson,
                     "{\"cpu_utilization\": 0.7, \"application_utilization\": 0.8, "
                     "\"mem_utilization\": 0.9, \"rps_fractional\": 1000, \"eps\": 2,  "
                     "\"unknown_field\": 2,"
@@ -197,7 +191,7 @@ TEST(OrcaParserUtilTest, JsonHeaderUnknownField) {
 TEST(OrcaParserUtilTest, JsonHeaderIncorrectFieldType) {
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixJson(), "{\"cpu_utilization\": \"0.7\"")}};
+       absl::StrCat(kHeaderFormatPrefixJson, "{\"cpu_utilization\": \"0.7\"")}};
   EXPECT_THAT(parseOrcaLoadReportHeaders(headers),
               StatusHelpers::HasStatus(absl::StatusCode::kInvalidArgument,
                                        testing::HasSubstr("invalid JSON")));
@@ -225,7 +219,7 @@ TEST(OrcaParserUtilTest, BinaryHeader) {
       Envoy::Base64::encode(proto_string.c_str(), proto_string.length());
   Http::TestRequestHeaderMapImpl headers{
       {std::string(kEndpointLoadMetricsHeader),
-       absl::StrCat(formattedHeaderPrefixBin(), orca_load_report_header_bin)}};
+       absl::StrCat(kHeaderFormatPrefixBin, orca_load_report_header_bin)}};
   EXPECT_THAT(parseOrcaLoadReportHeaders(headers),
               StatusHelpers::IsOkAndHolds(ProtoEq(exampleOrcaLoadReport())));
 }