opentelemetry tracer: Not crash envoy when environment resource detec…

…tor can't detect attributes (#32229) * Not crash envoy when env resource detector can't detect attributes Signed-off-by: Joao Grassi <[email protected]> * Add log warning for invalid env variable contents Signed-off-by: Joao Grassi <[email protected]> --------- Signed-off-by: Joao Grassi <[email protected]>
envoyproxy · Feb 8, 2024 · b788e1a · b788e1a
1 parent d14ce54
commit b788e1a
Show file tree

Hide file tree

Showing 5 changed files with 116 additions and 24 deletions.
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -40,6 +40,9 @@ bug_fixes:
     fixed a bug where second HTTP response headers received would cause Envoy to crash in cases where
     ``propagate_response_headers`` and retry configurations are enabled at the same time, and an upstream
     request is retried multiple times.
+- area: tracing
+  change: |
+    Prevent Envoy from crashing at start up when the OpenTelemetry environment resource detector cannot detect any attributes.
 
 removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

diff --git a/...ons/tracers/opentelemetry/resource_detectors/environment/environment_resource_detector.cc b/...ons/tracers/opentelemetry/resource_detectors/environment/environment_resource_detector.cc
@@ -29,22 +29,22 @@ Resource EnvironmentResourceDetector::detect() {
   resource.schema_url_ = "";
   std::string attributes_str = "";
 
-  attributes_str = Config::DataSource::read(ds, true, context_.serverFactoryContext().api());
+  TRY_NEEDS_AUDIT {
+    attributes_str = Config::DataSource::read(ds, true, context_.serverFactoryContext().api());
+  }
+  END_TRY catch (const EnvoyException& e) {
+    ENVOY_LOG(warn, "Failed to detect resource attributes from the environment: {}.", e.what());
+  }
 
   if (attributes_str.empty()) {
-    throw EnvoyException(
-        fmt::format("The OpenTelemetry environment resource detector is configured but the '{}'"
-                    " environment variable is empty.",
-                    kOtelResourceAttributesEnv));
+    return resource;
   }
 
   for (const auto& pair : StringUtil::splitToken(attributes_str, ",")) {
     const auto keyValue = StringUtil::splitToken(pair, "=");
     if (keyValue.size() != 2) {
-      throw EnvoyException(
-          fmt::format("The OpenTelemetry environment resource detector is configured but the '{}'"
-                      " environment variable has an invalid format.",
-                      kOtelResourceAttributesEnv));
+      ENVOY_LOG(warn, "Invalid resource format from the environment, invalid text: {}.", pair);
+      continue;
     }
 
     const std::string key = std::string(keyValue[0]);

diff --git a/test/extensions/tracers/opentelemetry/resource_detectors/environment/BUILD b/test/extensions/tracers/opentelemetry/resource_detectors/environment/BUILD
@@ -35,3 +35,19 @@ envoy_extension_cc_test(
         "@envoy_api//envoy/extensions/tracers/opentelemetry/resource_detectors/v3:pkg_cc_proto",
     ],
 )
+
+envoy_extension_cc_test(
+    name = "environment_resource_detector_integration_test",
+    srcs = [
+        "environment_resource_detector_integration_test.cc",
+    ],
+    extension_names = ["envoy.tracers.opentelemetry.resource_detectors.environment"],
+    deps = [
+        "//source/exe:main_common_lib",
+        "//source/extensions/tracers/opentelemetry:config",
+        "//source/extensions/tracers/opentelemetry/resource_detectors/environment:config",
+        "//test/integration:http_integration_lib",
+        "//test/test_common:utility_lib",
+        "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto",
+    ],
+)
diff --git a/...elemetry/resource_detectors/environment/environment_resource_detector_integration_test.cc b/...elemetry/resource_detectors/environment/environment_resource_detector_integration_test.cc
@@ -0,0 +1,76 @@
+#include <memory>
+#include <string>
+
+#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h"
+
+#include "test/integration/http_integration.h"
+#include "test/test_common/utility.h"
+
+#include "absl/strings/match.h"
+#include "absl/strings/string_view.h"
+#include "gtest/gtest.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace Tracers {
+namespace OpenTelemetry {
+namespace {
+
+class EnvironmentResourceDetectorIntegrationTest
+    : public Envoy::HttpIntegrationTest,
+      public testing::TestWithParam<Network::Address::IpVersion> {
+public:
+  EnvironmentResourceDetectorIntegrationTest()
+      : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam()) {
+
+    const std::string yaml_string = R"EOF(
+  provider:
+    name: envoy.tracers.opentelemetry
+    typed_config:
+      "@type": type.googleapis.com/envoy.config.trace.v3.OpenTelemetryConfig
+      grpc_service:
+        envoy_grpc:
+          cluster_name: opentelemetry_collector
+        timeout: 0.250s
+      service_name: "a_service_name"
+      resource_detectors:
+        - name: envoy.tracers.opentelemetry.resource_detectors.environment
+          typed_config:
+            "@type": type.googleapis.com/envoy.extensions.tracers.opentelemetry.resource_detectors.v3.EnvironmentResourceDetectorConfig
+  )EOF";
+
+    auto tracing_config =
+        std::make_unique<::envoy::extensions::filters::network::http_connection_manager::v3::
+                             HttpConnectionManager_Tracing>();
+    TestUtility::loadFromYaml(yaml_string, *tracing_config.get());
+    config_helper_.addConfigModifier(
+        [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
+                hcm) -> void { hcm.set_allocated_tracing(tracing_config.release()); });
+
+    initialize();
+    codec_client_ = makeHttpConnection(lookupPort("http"));
+  }
+};
+
+INSTANTIATE_TEST_SUITE_P(IpVersions, EnvironmentResourceDetectorIntegrationTest,
+                         testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
+                         TestUtility::ipTestParamsToString);
+
+// Verify Envoy starts even when the Environment resource detector wasn't able to detect any
+// attributes (env variable OTEL_RESOURCE_ATTRIBUTES not set)
+TEST_P(EnvironmentResourceDetectorIntegrationTest, TestWithNoEnvVariableSet) {
+  Http::TestRequestHeaderMapImpl request_headers{
+      {":method", "GET"}, {":path", "/test/long/url"}, {":scheme", "http"}, {":authority", "host"}};
+
+  auto response = sendRequestAndWaitForResponse(request_headers, 0, default_response_headers_, 0);
+
+  ASSERT_TRUE(response->waitForEndStream());
+  EXPECT_TRUE(response->complete());
+  EXPECT_EQ(response->headers().getStatusValue(), "200");
+}
+
+} // namespace
+} // namespace OpenTelemetry
+} // namespace Tracers
+} // namespace Extensions
+} // namespace Envoy
diff --git a/...racers/opentelemetry/resource_detectors/environment/environment_resource_detector_test.cc b/...racers/opentelemetry/resource_detectors/environment/environment_resource_detector_test.cc
@@ -29,8 +29,10 @@ TEST(EnvironmentResourceDetectorTest, EnvVariableNotPresent) {
       EnvironmentResourceDetectorConfig config;
 
   auto detector = std::make_unique<EnvironmentResourceDetector>(config, context);
-  EXPECT_THROW_WITH_MESSAGE(detector->detect(), EnvoyException,
-                            "Environment variable doesn't exist: OTEL_RESOURCE_ATTRIBUTES");
+  Resource resource = detector->detect();
+
+  EXPECT_EQ(resource.schema_url_, "");
+  EXPECT_TRUE(resource.attributes_.empty());
 }
 
 // Test detector when env variable is present but contains an empty value
@@ -43,16 +45,10 @@ TEST(EnvironmentResourceDetectorTest, EnvVariablePresentButEmpty) {
       EnvironmentResourceDetectorConfig config;
 
   auto detector = std::make_unique<EnvironmentResourceDetector>(config, context);
+  Resource resource = detector->detect();
 
-#ifdef WIN32
-  EXPECT_THROW_WITH_MESSAGE(detector->detect(), EnvoyException,
-                            "Environment variable doesn't exist: OTEL_RESOURCE_ATTRIBUTES");
-#else
-  EXPECT_THROW_WITH_MESSAGE(detector->detect(), EnvoyException,
-                            "The OpenTelemetry environment resource detector is configured but the "
-                            "'OTEL_RESOURCE_ATTRIBUTES'"
-                            " environment variable is empty.");
-#endif
+  EXPECT_EQ(resource.schema_url_, "");
+  EXPECT_TRUE(resource.attributes_.empty());
 }
 
 // Test detector with valid values in the env variable
@@ -96,11 +92,12 @@ TEST(EnvironmentResourceDetectorTest, EnvVariablePresentAndWithAttributesWrongFo
       EnvironmentResourceDetectorConfig config;
 
   auto detector = std::make_unique<EnvironmentResourceDetector>(config, context);
+  Resource resource = detector->detect();
 
-  EXPECT_THROW_WITH_MESSAGE(detector->detect(), EnvoyException,
-                            "The OpenTelemetry environment resource detector is configured but the "
-                            "'OTEL_RESOURCE_ATTRIBUTES'"
-                            " environment variable has an invalid format.");
+  EXPECT_EQ(resource.schema_url_, "");
+  EXPECT_EQ(1, resource.attributes_.size());
+  EXPECT_EQ(resource.attributes_.begin()->first, resource.attributes_.begin()->first);
+  EXPECT_EQ(resource.attributes_.begin()->second, resource.attributes_.begin()->second);
 }
 
 } // namespace OpenTelemetry