Merge branch 'main' into add_session_dfp2

Signed-off-by: ohadvano <[email protected]>

.azure-pipelines/stage/prechecks.yml

@@ -48,8 +48,9 @@ jobs:
         CI_TARGET: "format"
       protobuf:
         CI_TARGET: "check_and_fix_proto_format"
-      publishing:
-        CI_TARGET: docs
+      ${{ if eq(variables['Build.Reason'], 'PullRequest') }}:
+        publishing:
+          CI_TARGET: docs
   steps:
   - template: ../bazel.yml
     parameters:

api/envoy/config/trace/v3/skywalking.proto

@@ -22,9 +22,9 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#protodoc-title: SkyWalking tracer]
 
 // Configuration for the SkyWalking tracer. Please note that if SkyWalking tracer is used as the
-// provider of http tracer, then
-// :ref:`start_child_span <envoy_v3_api_field_extensions.filters.http.router.v3.Router.start_child_span>`
-// in the router must be set to true to get the correct topology and tracing data. Moreover, SkyWalking
+// provider of tracing, then
+// :ref:`spawn_upstream_span <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.spawn_upstream_span>`
+// in the tracing config must be set to true to get the correct topology and tracing data. Moreover, SkyWalking
 // Tracer does not support SkyWalking extension header (``sw8-x``) temporarily.
 // [#extension: envoy.tracers.skywalking]
 message SkyWalkingConfig {

api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto

@@ -7,6 +7,8 @@ import "envoy/extensions/common/ratelimit/v3/ratelimit.proto";
 import "envoy/type/v3/http_status.proto";
 import "envoy/type/v3/token_bucket.proto";
 
+import "google/protobuf/wrappers.proto";
+
 import "udpa/annotations/status.proto";
 import "validate/validate.proto";
 
@@ -20,7 +22,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // Local Rate limit :ref:`configuration overview <config_http_filters_local_rate_limit>`.
 // [#extension: envoy.filters.http.local_ratelimit]
 
-// [#next-free-field: 14]
+// [#next-free-field: 15]
 message LocalRateLimit {
   // The human readable prefix to use when emitting stats.
   string stat_prefix = 1 [(validate.rules).string = {min_len: 1}];
@@ -117,4 +119,10 @@ message LocalRateLimit {
   // Specifies if the local rate limit filter should include the virtual host rate limits.
   common.ratelimit.v3.VhRateLimitsOptions vh_rate_limits = 13
       [(validate.rules).enum = {defined_only: true}];
+
+  // Specifies if default token bucket should be always consumed.
+  // If set to false, default token bucket will only be consumed when there is
+  // no matching descriptor. If set to true, default token bucket will always
+  // be consumed. Default is true.
+  google.protobuf.BoolValue always_consume_default_token_bucket = 14;
 }

api/envoy/extensions/filters/http/router/v3/BUILD

@@ -6,6 +6,7 @@ licenses(["notice"])  # Apache 2
 
 api_proto_package(
     deps = [
+        "//envoy/annotations:pkg",
         "//envoy/config/accesslog/v3:pkg",
         "//envoy/extensions/filters/network/http_connection_manager/v3:pkg",
         "@com_github_cncf_udpa//udpa/annotations:pkg",

api/envoy/extensions/filters/http/router/v3/router.proto

@@ -8,6 +8,7 @@ import "envoy/extensions/filters/network/http_connection_manager/v3/http_connect
 import "google/protobuf/duration.proto";
 import "google/protobuf/wrappers.proto";
 
+import "envoy/annotations/deprecation.proto";
 import "udpa/annotations/status.proto";
 import "udpa/annotations/versioning.proto";
 import "validate/validate.proto";
@@ -52,7 +53,13 @@ message Router {
   // useful in scenarios where other filters (auth, ratelimit, etc.) make
   // outbound calls and have child spans rooted at the same ingress
   // parent. Defaults to false.
-  bool start_child_span = 2;
+  //
+  // .. attention::
+  //   This field is deprecated by the
+  //   :ref:`spawn_upstream_span <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.spawn_upstream_span>`.
+  //   Please use that ``spawn_upstream_span`` field to control the span creation.
+  bool start_child_span = 2
+      [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"];
 
   // Configuration for HTTP upstream logs emitted by the router. Upstream logs
   // are configured in the same way as access logs, but each log entry represents

api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto

@@ -130,7 +130,7 @@ message HttpConnectionManager {
     UNESCAPE_AND_FORWARD = 4;
   }
 
-  // [#next-free-field: 10]
+  // [#next-free-field: 11]
   message Tracing {
     option (udpa.annotations.versioning).previous_message_type =
         "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager.Tracing";
@@ -195,6 +195,27 @@ message HttpConnectionManager {
     //   Such a constraint is inherent to OpenCensus itself. It cannot be overcome without changes
     //   on OpenCensus side.
     config.trace.v3.Tracing.Http provider = 9;
+
+    // Create separate tracing span for each upstream request if true. And if this flag is set to true,
+    // the tracing provider will assume that Envoy will be independent hop in the trace chain and may
+    // set span type to client or server based on this flag.
+    // This will deprecate the
+    // :ref:`start_child_span <envoy_v3_api_field_extensions.filters.http.router.v3.Router.start_child_span>`
+    // in the router.
+    //
+    // Users should set appropriate value based on their tracing provider and actual scenario:
+    //
+    // * If Envoy is used as sidecar and users want to make the sidecar and its application as only one
+    //   hop in the trace chain, this flag should be set to false. And please also make sure the
+    //   :ref:`start_child_span <envoy_v3_api_field_extensions.filters.http.router.v3.Router.start_child_span>`
+    //   in the router is not set to true.
+    // * If Envoy is used as gateway or independent proxy, or users want to make the sidecar and its
+    //   application as different hops in the trace chain, this flag should be set to true.
+    // * If tracing provider that has explicit requirements on span creation (like SkyWalking),
+    //   this flag should be set to true.
+    //
+    // The default value is false for now for backward compatibility.
+    google.protobuf.BoolValue spawn_upstream_span = 10;
   }
 
   message InternalAddressConfig {

changelogs/current.yaml

@@ -95,6 +95,11 @@ minor_behavior_changes:
     The redis network filter :ref:`connection_rate_limit_per_sec
     <envoy_v3_api_field_extensions.filters.network.redis_proxy.v3.RedisProxy.ConnectionRateLimit.connection_rate_limit_per_sec>`
     must be greater than 0. A config that sets this value to 0 will be rejected.
+- area: local_rate_limit
+  change: |
+    Added new configuration field :ref:`always_consume_default_token_bucket
+    <envoy_v3_api_field_extensions.filters.http.local_ratelimit.v3.LocalRateLimit.always_consume_default_token_bucket>`
+    to allow for setting if default token bucket should be always consumed or only be consumed when there is no matching descriptor.
 
 bug_fixes:
 # *Changes expected to improve the state of the world and are unlikely to have negative effects*
@@ -295,6 +300,11 @@ new_features:
   change: |
     Added the ability to specify a custom upstream local address selector using
     :ref:`local_address_selector:<envoy_v3_api_field_config.core.v3.BindConfig.local_address_selector>`.
+- area: tracing
+  change: |
+    Added :ref:`spawn_upstream_span
+    <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.spawn_upstream_span>`
+    to control whether to create separate upstream span for upstream request.
 - area: original_dst
   change: |
     added support for the internal listener address recovery using the original destination listener filter.
@@ -306,3 +316,10 @@ deprecated:
 - area: tracing
   change: |
     Opencensus is deprecated and will be removed at version 1.30, since the upstream project has been abandoned.
+- area: tracing
+  change: |
+    :ref:`start_child_span <envoy_v3_api_field_extensions.filters.http.router.v3.Router.start_child_span>`
+    is deprecated by
+    :ref:`spawn_upstream_span
+    <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.spawn_upstream_span>`.
+    Please use the new field to control whether to create separate upstream span for upstream request.

ci/format_pre.sh

@@ -57,7 +57,7 @@ CURRENT=spelling
 "${ENVOY_SRCDIR}/tools/spelling/check_spelling_pedantic.py" --mark check
 
 # TODO(phlax): move clang/buildifier checks to bazel rules (/aspects)
-if [[ -n "$AZP_BRANCH" ]]; then
+if [[ -n "$CI_BRANCH" ]]; then
     CURRENT=check_format_test
     "${ENVOY_SRCDIR}/tools/code_format/check_format_test_helper.sh" --log=WARN
 fi

ci/run_envoy_docker.sh

@@ -117,8 +117,6 @@ fi
 docker run --rm \
        "${ENVOY_DOCKER_OPTIONS[@]}" \
        "${VOLUMES[@]}" \
-       -e AZP_BRANCH \
-       -e AZP_COMMIT_SHA \
        -e HTTP_PROXY \
        -e HTTPS_PROXY \
        -e NO_PROXY \

contrib/generic_proxy/filters/network/source/proxy.cc

@@ -78,6 +78,11 @@ uint32_t ActiveStream::maxPathTagLength() const {
   return connection_manager_tracing_config_->maxPathTagLength();
 }
 
+bool ActiveStream::spawnUpstreamSpan() const {
+  ASSERT(connection_manager_tracing_config_.has_value());
+  return connection_manager_tracing_config_->spawnUpstreamSpan();
+}
+
 Envoy::Event::Dispatcher& ActiveStream::dispatcher() {
   return parent_.downstreamConnection().dispatcher();
 }

contrib/generic_proxy/filters/network/source/proxy.h

@@ -281,6 +281,7 @@ class ActiveStream : public FilterChainManager,
   const Tracing::CustomTagMap* customTags() const override;
   bool verbose() const override;
   uint32_t maxPathTagLength() const override;
+  bool spawnUpstreamSpan() const override;
 
   bool active_stream_reset_{false};
 

docs/build.sh

@@ -20,7 +20,7 @@ DEV_VERSION_REGEX="-dev$"
 BUILD_TYPE=html
 
 if [[ "$VERSION" =~ $DEV_VERSION_REGEX ]]; then
-   if [[ "$AZP_BRANCH" == "$MAIN_BRANCH" ]]; then
+   if [[ "$CI_BRANCH" == "$MAIN_BRANCH" ]]; then
        # no need to build html, just rst
        BUILD_TYPE=rst
    fi
@@ -37,15 +37,6 @@ IFS=" " read -ra BAZEL_STARTUP_OPTIONS <<< "${BAZEL_STARTUP_OPTION_LIST:-}"
 # We want the binary at the end
 BAZEL_BUILD_OPTIONS+=(--remote_download_toplevel)
 
-if [[ "${AZP_BRANCH}" =~ ^refs/pull ]]; then
-    # For PRs use the unmerged PR commit in the version string.
-    #
-    # Staged/built docs still use the merged sha in the URL to distinguish builds
-    #
-    export BUILD_DOCS_SHA="${AZP_COMMIT_SHA}"
-    BAZEL_BUILD_OPTIONS+=("--action_env=BUILD_DOCS_SHA")
-fi
-
 if [[ -n "${CI_TARGET_BRANCH}" ]] || [[ -n "${SPHINX_QUIET}" ]]; then
     export SPHINX_RUNNER_ARGS="-v warn"
     BAZEL_BUILD_OPTIONS+=("--action_env=SPHINX_RUNNER_ARGS")
@@ -56,7 +47,7 @@ if [[ "${BUILD_TYPE}" == "html" ]] || [[ -n "${DOCS_BUILD_HTML}" ]]; then
     BUILD_HTML=1
     BUILD_HTML_TARGET="//docs:html"
     BUILD_HTML_TARBALL="bazel-bin/docs/html.tar.gz"
-    if [[ -n "${AZP_BRANCH}" ]] || [[ -n "${DOCS_BUILD_RELEASE}" ]]; then
+    if [[ -n "${CI_BRANCH}" ]] || [[ -n "${DOCS_BUILD_RELEASE}" ]]; then
         # CI build - use git sha
         BUILD_HTML_TARGET="//docs:html_release"
         BUILD_HTML_TARBALL="bazel-bin/docs/html_release.tar.gz"

docs/root/intro/arch_overview/observability/tracing.rst

@@ -39,9 +39,6 @@ initiated:
 * Randomly sampled via the :ref:`random_sampling <config_http_conn_man_runtime_random_sampling>`
   runtime setting.
 
-The router filter is also capable of creating a child span for egress calls via the
-:ref:`start_child_span <envoy_v3_api_field_extensions.filters.http.router.v3.Router.start_child_span>` option.
-
 .. _arch_overview_tracing_context_propagation:
 
 Trace context propagation
@@ -163,3 +160,62 @@ Tracing providers have varying level of support for getting and setting baggage:
 * Lightstep (and any OpenTelemetry-compliant tracer) can read/write baggage
 * Zipkin support is not yet implemented
 * X-Ray and OpenCensus don't support baggage
+
+Different types of span
+-----------------------
+
+As mentioned in the previous paragraph, a trace is composed of one or more spans, which may have different types.
+Tracing systems such as SkyWalking, ZipKin, and OpenTelemetry, among others, offer the same or similar span types.
+The most common types are CLIENT and SERVER. A CLIENT type span is generated by a client for a request that is
+sent to a server, while a SERVER type span is generated by a server for a request that is received from a client.
+
+
+A basic trace chain looks like the following snippet. Typically, the parent span of a server span should be a
+client span. Every hop in the chain must ensure the correctness of the span type.
+
+.. code-block:: text
+
+   -> [SERVER -> CLIENT] -> [SERVER -> CLIENT] -> ...
+             App A                 App B
+
+
+Different modes of Envoy
+------------------------
+
+Because Envoy is widely used in the service mesh as sidecar, it is important to understand the different tracing
+modes of Envoy.
+
+
+In the first mode, Envoy is used as a sidecar. The sidecar and its associated application are treated as a single
+hop in the trace chain. If a tracing system with typed spans is used, the ideal trace chain might look like the
+following snippet.
+
+.. code-block:: text
+
+   -> [[SERVER (inbound sidecar) -> App -> CLIENT (outbound sidecar)]] -> ...
+                                    App
+
+
+As you can see, in the first mode, the inbound sidecar will always generate a SERVER span, and the outbound sidecar
+will always generate a CLIENT span. The application will not generate any spans but will only propagate the trace context.
+
+
+In the second mode, Envoy is used as a gateway. Or, Envoy can be used as a sidecar, but in this case,
+the sidecar and its application are treated as separate hops in the trace chain. If a tracing system with typed spans
+is used, the ideal trace chain might look like the following snippet.
+
+.. code-block:: text
+
+   -> [SERVER -> CLIENT] -> [SERVER -> CLIENT] -> [SERVER -> CLIENT] -> [SERVER -> CLIENT] -> ...
+           Gateway           Inbound Sidecar            App             Outbound Sidecar
+
+
+
+As you can see, in the second mode, Envoy will generate a SERVER span for downstream requests and a CLIENT span for
+upstream requests. The application may also generate spans for its own work.
+
+
+To enable this mode, please set
+:ref:`spawn_upstream_span <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.spawn_upstream_span>`
+to true explicitly. This tells the tracing provider to generate a CLIENT span for upstream requests and treat Envoy
+as an independent hop in the trace chain.

envoy/tracing/trace_config.h

@@ -21,6 +21,11 @@ class Config {
    */
   virtual OperationName operationName() const PURE;
 
+  /**
+   * @return create separated child span for upstream request if true.
+   */
+  virtual bool spawnUpstreamSpan() const PURE;
+
   /**
    * @return custom tags to be attached to the active span.
    */
@@ -80,6 +85,11 @@ class ConnectionManagerTracingConfig : public TracingConfig {
    */
   virtual OperationName operationName() const PURE;
 
+  /**
+   * @return create separated child span for upstream request if true.
+   */
+  virtual bool spawnUpstreamSpan() const PURE;
+
   /**
    * @return true if spans should be annotated with more detailed information.
    */

source/common/http/conn_manager_impl.cc

@@ -1917,6 +1917,11 @@ uint32_t ConnectionManagerImpl::ActiveStream::maxPathTagLength() const {
   return connection_manager_tracing_config_->max_path_tag_length_;
 }
 
+bool ConnectionManagerImpl::ActiveStream::spawnUpstreamSpan() const {
+  ASSERT(connection_manager_tracing_config_.has_value());
+  return connection_manager_tracing_config_->spawn_upstream_span_;
+}
+
 const Router::RouteEntry::UpgradeMap* ConnectionManagerImpl::ActiveStream::upgradeMap() {
   // We must check if the 'cached_route_' optional is populated since this function can be called
   // early via sendLocalReply(), before the cached route is populated.

source/common/http/conn_manager_impl.h

@@ -500,6 +500,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
     const Tracing::CustomTagMap* customTags() const override;
     bool verbose() const override;
     uint32_t maxPathTagLength() const override;
+    bool spawnUpstreamSpan() const override;
 
     std::shared_ptr<bool> still_alive_ = std::make_shared<bool>(true);
   };

source/common/router/upstream_request.cc

@@ -94,8 +94,8 @@ UpstreamRequest::UpstreamRequest(RouterFilterInterface& parent,
       stream_options_({can_send_early_data, can_use_http3}), grpc_rq_success_deferred_(false),
       upstream_wait_for_response_headers_before_disabling_read_(Runtime::runtimeFeatureEnabled(
           "envoy.reloadable_features.upstream_wait_for_response_headers_before_disabling_read")) {
-  if (parent_.config().start_child_span_) {
-    if (auto tracing_config = parent_.callbacks()->tracingConfig(); tracing_config.has_value()) {
+  if (auto tracing_config = parent_.callbacks()->tracingConfig(); tracing_config.has_value()) {
+    if (tracing_config->spawnUpstreamSpan() || parent_.config().start_child_span_) {
       span_ = parent_.callbacks()->activeSpan().spawnChild(
           tracing_config.value().get(),
           absl::StrCat("router ", parent.cluster()->observabilityName(), " egress"),

source/common/tracing/tracer_config_impl.h

@@ -50,6 +50,9 @@ class ConnectionManagerTracingConfigImpl : public ConnectionManagerTracingConfig
       break;
     }
 
+    spawn_upstream_span_ =
+        PROTOBUF_GET_WRAPPED_OR_DEFAULT(tracing_config, spawn_upstream_span, false);
+
     for (const auto& tag : tracing_config.custom_tags()) {
       custom_tags_.emplace(tag.tag(), Tracing::CustomTagUtility::createCustomTag(tag));
     }
@@ -101,6 +104,7 @@ class ConnectionManagerTracingConfigImpl : public ConnectionManagerTracingConfig
   Tracing::OperationName operationName() const override { return operation_name_; }
   bool verbose() const override { return verbose_; }
   uint32_t maxPathTagLength() const override { return max_path_tag_length_; }
+  bool spawnUpstreamSpan() const override { return spawn_upstream_span_; }
 
   // TODO(wbpcode): keep this field be public for compatibility. Then the HCM needn't change much
   // code to use this config.
@@ -111,6 +115,7 @@ class ConnectionManagerTracingConfigImpl : public ConnectionManagerTracingConfig
   envoy::type::v3::FractionalPercent overall_sampling_;
   bool verbose_{};
   uint32_t max_path_tag_length_{};
+  bool spawn_upstream_span_{};
 };
 
 } // namespace Tracing

source/common/tracing/tracer_impl.h

@@ -55,6 +55,8 @@ class EgressConfigImpl : public Config {
   const CustomTagMap* customTags() const override { return nullptr; }
   bool verbose() const override { return false; }
   uint32_t maxPathTagLength() const override { return Tracing::DefaultMaxPathTagLength; }
+  // This EgressConfigImpl is only used for async client tracing. Return false here is OK.
+  bool spawnUpstreamSpan() const override { return false; }
 };
 
 using EgressConfig = ConstSingleton<EgressConfigImpl>;