Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cluster] Use alt_stat_name for general observability purposes (access log, tracing, admin) #15139

Merged
merged 10 commits into from
Mar 2, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion api/envoy/admin/v3/clusters.proto
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ message Clusters {
}

// Details an individual cluster's current status.
// [#next-free-field: 7]
// [#next-free-field: 8]
message ClusterStatus {
option (udpa.annotations.versioning).previous_message_type = "envoy.admin.v2alpha.ClusterStatus";

Expand Down Expand Up @@ -80,6 +80,9 @@ message ClusterStatus {

// :ref:`Circuit breaking <arch_overview_circuit_break>` settings of the cluster.
config.cluster.v3.CircuitBreakers circuit_breakers = 6;

// Observability name of the cluster.
string observability_name = 7;
}

// Current state of a particular host.
Expand Down
5 changes: 4 additions & 1 deletion api/envoy/admin/v4alpha/clusters.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 11 additions & 5 deletions api/envoy/config/cluster/v3/cluster.proto
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import "google/protobuf/wrappers.proto";

import "xds/core/v3/collection_entry.proto";

import "udpa/annotations/migrate.proto";
import "udpa/annotations/security.proto";
import "udpa/annotations/status.proto";
import "udpa/annotations/versioning.proto";
Expand Down Expand Up @@ -683,11 +684,16 @@ message Cluster {
// Any ``:`` in the cluster name will be converted to ``_`` when emitting statistics.
string name = 1 [(validate.rules).string = {min_len: 1}];

// An optional alternative to the cluster name to be used while emitting stats.
// Any ``:`` in the name will be converted to ``_`` when emitting statistics. This should not be
// confused with :ref:`Router Filter Header
// <config_http_filters_router_x-envoy-upstream-alt-stat-name>`.
string alt_stat_name = 28;
// An optional alternative to the cluster name to be used for observability. This name is used
// emitting stats for the cluster and access logging the cluster name. This will appear as
// additional information in configuration dumps of a cluster's current status as
// :ref:`observability_name <envoy_v3_api_field_admin.v3.ClusterStatus.observability_name>`
// and as an additional tag "upstream_cluster.name" while tracing. Note: access logging using
// this field is presently enabled with runtime feature
// `envoy.reloadable_features.use_observable_cluster_name`. Any ``:`` in the name will be
// converted to ``_`` when emitting statistics. This should not be confused with :ref:`Router
// Filter Header <config_http_filters_router_x-envoy-upstream-alt-stat-name>`.
string alt_stat_name = 28 [(udpa.annotations.field_migrate).rename = "observability_name"];

oneof cluster_discovery_type {
// The :ref:`service discovery type <arch_overview_service_discovery_types>`
Expand Down
15 changes: 10 additions & 5 deletions api/envoy/config/cluster/v4alpha/cluster.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions docs/root/configuration/observability/access_log/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,9 @@ The following command operators are supported:
%UPSTREAM_HOST%
Upstream host URL (e.g., tcp://ip:port for TCP connections).

%UPSTREAM_CLUSTER%
Upstream cluster to which the upstream host belongs to.
%UPSTREAM_CLUSTER% Upstream cluster to which the upstream host belongs to. If runtime feature
`envoy.reloadable_features.use_observable_cluster_name` is enabled, then :ref:`alt_stat_name
<envoy_v3_api_field_config.cluster.v3.Cluster.alt_stat_name>` will be used if provided.

%UPSTREAM_LOCAL_ADDRESS%
Local address of the upstream connection. If the address is an IP address it includes both
Expand Down
2 changes: 1 addition & 1 deletion docs/root/intro/arch_overview/observability/tracing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ associated with it. Each span generated by Envoy contains the following data:
* HTTP request URL, method, protocol and user-agent.
* Additional custom tags set via :ref:`custom_tags
<envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.custom_tags>`.
* Upstream cluster name and address.
* Upstream cluster name, observability name, and address.
* HTTP response status code.
* GRPC response status and message (if available).
* An error tag when HTTP status is 5xx or GRPC status is not "OK".
Expand Down
3 changes: 3 additions & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ Minor Behavior Changes
----------------------
*Changes that may cause incompatibilities for some users, but should not for most*

* access_logs: change command operator %UPSTREAM_CLUSTER% to resolve to :ref:`alt_stat_name <envoy_v3_api_field_config.cluster.v3.Cluster.alt_stat_name>` if provided. This behavior can be reverted by disabling the runtime feature `envoy.reloadable_features.use_observable_cluster_name`.
* admin: added :ref:`observability_name <envoy_v3_api_field_admin.v3.ClusterStatus.observability_name>` information to GET /clusters?format=json :ref:`cluster status <envoy_v3_api_msg_admin.v3.ClusterStatus>`.
* dns: both the :ref:`strict DNS <arch_overview_service_discovery_types_strict_dns>` and
:ref:`logical DNS <arch_overview_service_discovery_types_logical_dns>` cluster types now honor the
:ref:`hostname <envoy_v3_api_field_config.endpoint.v3.Endpoint.hostname>` field if not empty.
Expand Down Expand Up @@ -46,6 +48,7 @@ Minor Behavior Changes
* oauth filter: added the optional parameter :ref:`auth_scopes <envoy_v3_api_field_extensions.filters.http.oauth2.v3alpha.OAuth2Config.auth_scopes>` with default value of 'user' if not provided. Enables this value to be overridden in the Authorization request to the OAuth provider.
* perf: allow reading more bytes per operation from raw sockets to improve performance.
* router: extended custom date formatting to DOWNSTREAM_PEER_CERT_V_START and DOWNSTREAM_PEER_CERT_V_END when using :ref:`custom request/response header formats <config_http_conn_man_headers_custom_request_headers>`.
* tracing: added `upstream_address.name` tag that resolves to resolve to :ref:`alt_stat_name <envoy_v3_api_field_config.cluster.v3.Cluster.alt_stat_name>` if provided (and otherwise the cluster name).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* tracing: added `upstream_address.name` tag that resolves to resolve to :ref:`alt_stat_name <envoy_v3_api_field_config.cluster.v3.Cluster.alt_stat_name>` if provided (and otherwise the cluster name).
* tracing: added `upstream_cluster.name` tag that resolves to resolve to :ref:`alt_stat_name <envoy_v3_api_field_config.cluster.v3.Cluster.alt_stat_name>` if provided (and otherwise the cluster name).

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Thanks. Fixed

* upstream: host weight changes now cause a full load balancer rebuild as opposed to happening
atomically inline. This change has been made to support load balancer pre-computation of data
structures based on host weight, but may have performance implications if host weight changes
Expand Down
5 changes: 4 additions & 1 deletion generated_api_shadow/envoy/admin/v3/clusters.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion generated_api_shadow/envoy/admin/v4alpha/clusters.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 11 additions & 5 deletions generated_api_shadow/envoy/config/cluster/v3/cluster.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 10 additions & 5 deletions generated_api_shadow/envoy/config/cluster/v4alpha/cluster.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions include/envoy/upstream/upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,14 @@ class ClusterInfo {
*/
virtual const std::string& name() const PURE;

/**
* @return the observability name associated to the cluster. Used in stats, tracing, logging, and
* config dumps. The observability name is configured with :ref:`alt_stat_name
* <envoy_api_field_config.cluster.v3.Cluster.alt_stat_name>`. If unprovided, the default value is
* the cluster name.
*/
virtual const std::string& observabilityName() const PURE;

/**
* @return ResourceManager& the resource manager to use by proxy agents for this cluster (at
* a particular priority).
Expand Down
2 changes: 2 additions & 0 deletions source/common/formatter/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ envoy_cc_library(
deps = [
"//include/envoy/api:api_interface",
"//include/envoy/formatter:substitution_formatter_interface",
"//include/envoy/runtime:runtime_interface",
"//include/envoy/stream_info:stream_info_interface",
"//include/envoy/upstream:upstream_interface",
"//source/common/common:assert_lib",
Expand All @@ -25,6 +26,7 @@ envoy_cc_library(
"//source/common/grpc:common_lib",
"//source/common/http:utility_lib",
"//source/common/protobuf:message_validator_lib",
"//source/common/runtime:runtime_features_lib",
"//source/common/stream_info:utility_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
Expand Down
9 changes: 8 additions & 1 deletion source/common/formatter/substitution_formatter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "common/http/utility.h"
#include "common/protobuf/message_validator_impl.h"
#include "common/protobuf/utility.h"
#include "common/runtime/runtime_features.h"
#include "common/stream_info/utility.h"

#include "absl/strings/str_split.h"
Expand Down Expand Up @@ -756,7 +757,13 @@ StreamInfoFormatter::StreamInfoFormatter(const std::string& field_name) {
std::string upstream_cluster_name;
if (stream_info.upstreamClusterInfo().has_value() &&
stream_info.upstreamClusterInfo().value() != nullptr) {
upstream_cluster_name = stream_info.upstreamClusterInfo().value()->name();
if (Runtime::runtimeFeatureEnabled(
"envoy.reloadable_features.use_observable_cluster_name")) {
upstream_cluster_name =
stream_info.upstreamClusterInfo().value()->observabilityName();
} else {
upstream_cluster_name = stream_info.upstreamClusterInfo().value()->name();
}
}

return upstream_cluster_name.empty()
Expand Down
1 change: 1 addition & 0 deletions source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ constexpr const char* runtime_features[] = {
"envoy.reloadable_features.treat_host_like_authority",
"envoy.reloadable_features.treat_upstream_connect_timeout_as_connect_failure",
"envoy.reloadable_features.upstream_host_weight_change_causes_rebuild",
"envoy.reloadable_features.use_observable_cluster_name",
"envoy.reloadable_features.vhds_heartbeats",
"envoy.reloadable_features.unify_grpc_handling",
"envoy.reloadable_features.upstream_http2_flood_checks",
Expand Down
2 changes: 2 additions & 0 deletions source/common/tracing/http_tracer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ void HttpTracerUtility::setCommonTags(Span& span, const Http::ResponseHeaderMap*

if (nullptr != stream_info.upstreamHost()) {
span.setTag(Tracing::Tags::get().UpstreamCluster, stream_info.upstreamHost()->cluster().name());
span.setTag(Tracing::Tags::get().UpstreamClusterName,
stream_info.upstreamHost()->cluster().observabilityName());
}

// Post response data.
Expand Down
1 change: 1 addition & 0 deletions source/common/tracing/http_tracer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class TracingTagValues {
const std::string Status = "status";
const std::string UpstreamAddress = "upstream_address";
const std::string UpstreamCluster = "upstream_cluster";
const std::string UpstreamClusterName = "upstream_cluster.name";
const std::string UserAgent = "user_agent";
const std::string Zone = "zone";

Expand Down
4 changes: 3 additions & 1 deletion source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,9 @@ ClusterInfoImpl::ClusterInfoImpl(
const envoy::config::core::v3::BindConfig& bind_config, Runtime::Loader& runtime,
TransportSocketMatcherPtr&& socket_matcher, Stats::ScopePtr&& stats_scope, bool added_via_api,
Server::Configuration::TransportSocketFactoryContext& factory_context)
: runtime_(runtime), name_(config.name()), type_(config.type()),
: runtime_(runtime), name_(config.name()),
observability_name_(PROTOBUF_GET_STRING_OR_DEFAULT(config, alt_stat_name, name_)),
type_(config.type()),
extension_protocol_options_(parseExtensionProtocolOptions(config, factory_context)),
http_protocol_options_(
createOptions(config, extensionProtocolOptionsTyped<HttpProtocolOptionsConfigImpl>(
Expand Down
2 changes: 2 additions & 0 deletions source/common/upstream/upstream_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,7 @@ class ClusterInfoImpl : public ClusterInfo,
uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; }
uint32_t maxResponseHeadersCount() const override { return max_response_headers_count_; }
const std::string& name() const override { return name_; }
const std::string& observabilityName() const override { return observability_name_; }
ResourceManager& resourceManager(ResourcePriority priority) const override;
TransportSocketMatcher& transportSocketMatcher() const override { return *socket_matcher_; }
ClusterStats& stats() const override { return stats_; }
Expand Down Expand Up @@ -676,6 +677,7 @@ class ClusterInfoImpl : public ClusterInfo,

Runtime::Loader& runtime_;
const std::string name_;
const std::string observability_name_;
const envoy::config::cluster::v3::Cluster::DiscoveryType type_;
const absl::flat_hash_map<std::string, ProtocolOptionsConfigConstSharedPtr>
extension_protocol_options_;
Expand Down
3 changes: 3 additions & 0 deletions source/server/admin/clusters_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ void ClustersHandler::writeClustersAsJson(Buffer::Instance& response) {

envoy::admin::v3::ClusterStatus& cluster_status = *clusters.add_cluster_statuses();
cluster_status.set_name(cluster_info->name());
cluster_status.set_observability_name(cluster_info->observabilityName());

addCircuitBreakerSettingsAsJson(
envoy::config::core::v3::RoutingPriority::DEFAULT,
Expand Down Expand Up @@ -201,6 +202,8 @@ void ClustersHandler::writeClustersAsText(Buffer::Instance& response) {
UNREFERENCED_PARAMETER(name);
const Upstream::Cluster& cluster = cluster_ref.get();
const std::string& cluster_name = cluster.info()->name();
response.add(fmt::format("{}::observability_name::{}\n", cluster_name,
cluster.info()->observabilityName()));
addOutlierInfo(cluster_name, cluster.outlierDetector(), response);

addCircuitBreakerSettingsAsText(
Expand Down
1 change: 1 addition & 0 deletions test/common/formatter/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ envoy_cc_test(
"//test/mocks/ssl:ssl_mocks",
"//test/mocks/stream_info:stream_info_mocks",
"//test/mocks/upstream:cluster_info_mocks",
"//test/test_common:test_runtime_lib",
"//test/test_common:threadsafe_singleton_injector_lib",
"//test/test_common:utility_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
Expand Down
Loading