ext_proc: change default sampling to false inheriting from parent dec…

…ision (#37794)

Commit Message: ext_proc: change default sampling decision to false
Fixes: #37783
Additional Description: Default to using the parent span's sampled
status instead of true (default in
[StreamOptions](https://github.com/envoyproxy/envoy/blob/df5260957fc9446b57114a74ffbdc5d9b5831bfd/envoy/http/async_client.h#L315-L316)),
so that we don't emit orphan spans for all ext_proc streams where the
parent span is not sampled. That's the same issue seen in Lua httpCall
(#33200) and ext_authz
(#19343).


Risk Level: Low
Testing: changed the tracing integration test behavior and validated in
production.

---------

Signed-off-by: Fernando Cainelli <[email protected]>

changelogs/current.yaml

@@ -2,6 +2,10 @@ date: Pending
 
 behavior_changes:
 # *Changes that are expected to cause an incompatibility if applicable; deployment changes are likely required*
+- area: ext_proc
+  change: |
+    Previously, tracing spans generated by ``ext_proc`` were always sampled by default.
+     Now, the default sampling decision of an ``ext_proc`` span is inherited from the parent span.
 - area: tracing
   change: |
     Removed support for (long deprecated) opencensus tracing extension.

source/extensions/filters/http/ext_proc/ext_proc.cc

@@ -399,7 +399,8 @@ Filter::StreamOpenState Filter::openStream() {
     auto options = Http::AsyncClient::StreamOptions()
                        .setParentSpan(decoder_callbacks_->activeSpan())
                        .setParentContext(grpc_context)
-                       .setBufferBodyForRetry(grpc_service_.has_retry_policy());
+                       .setBufferBodyForRetry(grpc_service_.has_retry_policy())
+                       .setSampled(absl::nullopt);
 
     ExternalProcessorClient* grpc_client = dynamic_cast<ExternalProcessorClient*>(client_.get());
     ExternalProcessorStreamPtr stream_object =

test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc

@@ -862,7 +862,7 @@ TEST_P(ExtProcIntegrationTest, GetAndCloseStreamWithTracing) {
     ext_proc_span.set_operation_name(
         "async envoy.service.ext_proc.v3.ExternalProcessor.Process egress");
     ext_proc_span.set_context_injected(true);
-    ext_proc_span.set_sampled(true);
+    ext_proc_span.set_sampled(false);
     ext_proc_span.mutable_tags()->insert({"grpc.status_code", "0"});
     ext_proc_span.mutable_tags()->insert({"upstream_cluster", "ext_proc_server_0"});
     if (IsEnvoyGrpc()) {
@@ -928,7 +928,7 @@ TEST_P(ExtProcIntegrationTest, GetAndFailStreamWithTracing) {
     ext_proc_span.set_operation_name(
         "async envoy.service.ext_proc.v3.ExternalProcessor.Process egress");
     ext_proc_span.set_context_injected(true);
-    ext_proc_span.set_sampled(true);
+    ext_proc_span.set_sampled(false);
     ext_proc_span.mutable_tags()->insert({"grpc.status_code", "2"});
     ext_proc_span.mutable_tags()->insert({"error", "true"});
     ext_proc_span.mutable_tags()->insert({"upstream_cluster", "ext_proc_server_0"});
@@ -2554,7 +2554,7 @@ TEST_P(ExtProcIntegrationTest, RequestMessageTimeoutWithTracing) {
     ext_proc_span.set_operation_name(
         "async envoy.service.ext_proc.v3.ExternalProcessor.Process egress");
     ext_proc_span.set_context_injected(true);
-    ext_proc_span.set_sampled(true);
+    ext_proc_span.set_sampled(false);
     ext_proc_span.mutable_tags()->insert({"status", "canceled"});
     ext_proc_span.mutable_tags()->insert({"error", ""}); // not an error
     ext_proc_span.mutable_tags()->insert({"upstream_cluster", "ext_proc_server_0"});

test/extensions/filters/http/ext_proc/tracer_test_filter.cc

@@ -20,10 +20,10 @@ const Tracing::TraceContextHandler& traceParentHeader() {
 
 struct ExpectedSpan {
   std::string operation_name;
-  bool sampled;
-  bool context_injected;
+  bool sampled{false};
+  bool context_injected{false};
   std::map<std::string, std::string> tags;
-  bool tested;
+  bool tested{false};
 };
 
 using ExpectedSpansSharedPtr = std::shared_ptr<std::vector<ExpectedSpan>>;
@@ -116,9 +116,9 @@ class Span : public Tracing::Span {
   ExpectedSpansSharedPtr expected_spans_;
 
   std::map<std::string, std::string> tags_;
-  bool context_injected_;
-  bool sampled_;
-  bool finished_;
+  bool context_injected_{false};
+  bool sampled_{false};
+  bool finished_{false};
 };
 
 class Driver : public Tracing::Driver, Logger::Loggable<Logger::Id::tracing> {