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

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Feb 22, 2021

Commit Message: Re-purpose alt_stat_name for other observability use-cases: access logging, tracing, and admin dumps.
Additional Description:

  • A new field in ClusterInfo is added, observabilityName() that is the alt_stat_name if provided, and otherwise the Cluster name.
  • Access logs: %UPSTREAM_CLUSTER% resolves to observabilityName().
  • Tracing: a new tag upstream_cluster.name is added to reference observabilityName().
  • Admin: The observability name was already visible in config_dump in the cluster configuration. The ClusterStatus adds the observability_name field.

Risk Level: Low, just adds observability.
Testing: Added tests for access logging with/without runtime feature enabled.
Docs Changes: Added doc changes to Access Log page, Tracing page.
Release Notes: Added release notes for all three new uses cases.
Fixes: #14309
Runtime guard: Added runtime guard envoy.reloadable_features.use_observable_cluster_name that controls whether formatter %UPSTREAM_CLUSTER% resolves to original cluster name or alt_stat_name.
API considerations: Tagged alt_stat_name for general purpose rename observability_name.

@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
API shepherd assignee is @lizan
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #15139 was opened by asraa.

see: more, trace.

Signed-off-by: Asra Ali <[email protected]>
Signed-off-by: Asra Ali <[email protected]>
Signed-off-by: Asra Ali <[email protected]>
Signed-off-by: Asra Ali <[email protected]>
Signed-off-by: Asra Ali <[email protected]>
@asraa
Copy link
Contributor Author

asraa commented Feb 26, 2021

@zuercher could you please take a look or assign to someone else?

Also happy to split this up (although there is not too much complexity)

zuercher
zuercher previously approved these changes Feb 26, 2021
Copy link
Member

@zuercher zuercher left a comment

Choose a reason for hiding this comment

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

Looks good!

@asraa
Copy link
Contributor Author

asraa commented Mar 1, 2021

@envoyproxy/api-shepherds could you PTAL for API changes?

zuercher
zuercher previously approved these changes Mar 1, 2021
mattklein123
mattklein123 previously approved these changes Mar 1, 2021
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

API LGTM modulo one typo I found (I think).

@@ -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

@repokitteh-read-only repokitteh-read-only bot removed the api label Mar 1, 2021
Signed-off-by: Asra Ali <[email protected]>
@asraa asraa dismissed stale reviews from mattklein123 and zuercher via b902d55 March 1, 2021 18:00
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

API LGTM!

@repokitteh-read-only repokitteh-read-only bot removed the api label Mar 1, 2021
@asraa
Copy link
Contributor Author

asraa commented Mar 2, 2021

@zuercher sorry for the re-ping, the typo-fix dropped the LGTM but I think this is ready to merge!

@asraa asraa merged commit 5551e0a into envoyproxy:main Mar 2, 2021
zuercher pushed a commit that referenced this pull request Jan 18, 2022
#19475)

See #15139 ([cluster] Use alt_stat_name for general observability purposes (access log, tracing, admin)),
which introduced a runtime guarded feature, which has been enabled by default for  6 months, so remove
the old code path.

Risk Level: Low
Testing: n/a
Docs Changes: updated
Release Notes: Deprecate envoy.reloadable_features.use_observable_cluster_name.
Platform Specific Features: n/a
Signed-off-by: Loong <[email protected]>
joshperry pushed a commit to joshperry/envoy that referenced this pull request Feb 13, 2022
envoyproxy#19475)

See envoyproxy#15139 ([cluster] Use alt_stat_name for general observability purposes (access log, tracing, admin)),
which introduced a runtime guarded feature, which has been enabled by default for  6 months, so remove
the old code path.

Risk Level: Low
Testing: n/a
Docs Changes: updated
Release Notes: Deprecate envoy.reloadable_features.use_observable_cluster_name.
Platform Specific Features: n/a
Signed-off-by: Loong <[email protected]>
Signed-off-by: Josh Perry <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

alt_stat_name as a standardized observability label for clusters
4 participants