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

tracing: apply tracer provider configuration defined as part of http_connection_manager filter #10405

Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0561ccc
tracing: apply tracer provider configuration defined as part of `http…
yskopets Mar 16, 2020
6ba1f98
docs: update docs with regards to OpenCensus support and add deprecat…
yskopets Mar 25, 2020
0e36c6c
Merge remote-tracking branch 'upstream/master' into feature/use-trace…
yskopets Mar 25, 2020
b910b95
code review: add `[deprecated = true]` annotation to `v2.Bootstrap.tr…
yskopets Mar 25, 2020
7ca80c7
docs: update tracing-related proto docs
yskopets Mar 26, 2020
41013a4
unit tests: mark unit tests the involve `v2.Bootstrap.tracing` as `DE…
yskopets Mar 26, 2020
073b24e
unit tests: update example configurations to test fix failures when b…
yskopets Mar 26, 2020
0801eba
code review: remove extra comments
yskopets Apr 8, 2020
880824d
code review: rework comment
yskopets Apr 8, 2020
f079135
code review: rework comment
yskopets Apr 8, 2020
49e0ec4
Merge remote-tracking branch 'upstream/master' into feature/use-trace…
yskopets Apr 8, 2020
66b1118
v2 api freeze: rework deprecation of Bootstrap.tracing field
yskopets Apr 8, 2020
fbc2921
Merge remote-tracking branch 'upstream/master' into feature/use-trace…
yskopets Apr 8, 2020
6bb0bdb
code review: move deprecation and release notices into 1.15.0 release
yskopets Apr 8, 2020
fc6591d
code review: do not deprecate Bootstrap.tracing field in v2 api
yskopets Apr 8, 2020
2107e1f
code review: leave HCM.Tracing.provider field as unimplmeneted in v2 api
yskopets Apr 8, 2020
834740f
Revert "code review: leave HCM.Tracing.provider field as unimplmenete…
yskopets Apr 8, 2020
16722dc
code review: update docs for Bootstrap.tracing field in v2 api
yskopets Apr 8, 2020
220f429
ci: try to fix failing `docs` build
yskopets Apr 8, 2020
67668e4
code review: rename `test_data/server/zipkin_tracing.yaml` into `test…
yskopets Apr 10, 2020
b5348f9
code review: reword release notes
yskopets Apr 10, 2020
6642087
code review: depreate use of `config.trace.v3.Tracing`
yskopets Apr 10, 2020
ed40fac
ci: fix cross-link in the docs
yskopets Apr 10, 2020
e88c14f
Merge remote-tracking branch 'upstream/master' into feature/use-trace…
yskopets Apr 10, 2020
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
7 changes: 5 additions & 2 deletions api/envoy/config/bootstrap/v2/bootstrap.proto
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,11 @@ message Bootstrap {
// Optional watchdog configuration.
Watchdog watchdog = 8;

// Configuration for an external tracing provider. If not specified, no
// tracing will be performed.
// Configuration for an external tracing provider.
//
// .. attention::
// This field has been deprecated in favor of :ref:`HttpConnectionManager.Tracing.provider
// <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.Tracing.provider>`.
trace.v2.Tracing tracing = 9;

// Configuration for the runtime configuration provider (deprecated). If not
Expand Down
9 changes: 6 additions & 3 deletions api/envoy/config/bootstrap/v3/bootstrap.proto
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,12 @@ message Bootstrap {
// Optional watchdog configuration.
Watchdog watchdog = 8;

// Configuration for an external tracing provider. If not specified, no
// tracing will be performed.
trace.v3.Tracing tracing = 9;
// Configuration for an external tracing provider.
//
// .. attention::
// This field has been deprecated in favor of :ref:`HttpConnectionManager.Tracing.provider
// <envoy_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.provider>`.
trace.v3.Tracing tracing = 9 [deprecated = true];
Copy link
Member

Choose a reason for hiding this comment

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

One question before deprecating, do you have thoughts on #10576?

Copy link
Member Author

Choose a reason for hiding this comment

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

@htuch Sorry, didn't notice in time.

I've replied here.


// Configuration for the runtime configuration provider. If not
// specified, a “null” provider will be used which will result in all defaults
Expand Down
1 change: 0 additions & 1 deletion api/envoy/config/bootstrap/v4alpha/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ api_proto_package(
"//envoy/config/listener/v3:pkg",
"//envoy/config/metrics/v3:pkg",
"//envoy/config/overload/v3:pkg",
"//envoy/config/trace/v4alpha:pkg",
"//envoy/extensions/transport_sockets/tls/v4alpha:pkg",
"@com_github_cncf_udpa//udpa/annotations:pkg",
],
Expand Down
9 changes: 2 additions & 7 deletions api/envoy/config/bootstrap/v4alpha/bootstrap.proto
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import "envoy/config/core/v4alpha/socket_option.proto";
import "envoy/config/listener/v3/listener.proto";
import "envoy/config/metrics/v3/stats.proto";
import "envoy/config/overload/v3/overload.proto";
import "envoy/config/trace/v4alpha/trace.proto";
import "envoy/extensions/transport_sockets/tls/v4alpha/cert.proto";

import "google/protobuf/duration.proto";
Expand Down Expand Up @@ -85,9 +84,9 @@ message Bootstrap {
core.v4alpha.ApiConfigSource ads_config = 3;
}

reserved 10, 11;
reserved 10, 11, 9;

reserved "runtime";
reserved "runtime", "tracing";

// Node identity to present to the management server and for instance
// identification purposes (e.g. in generated headers).
Expand Down Expand Up @@ -129,10 +128,6 @@ message Bootstrap {
// Optional watchdog configuration.
Watchdog watchdog = 8;

// Configuration for an external tracing provider. If not specified, no
// tracing will be performed.
trace.v4alpha.Tracing tracing = 9;

// Configuration for the runtime configuration provider. If not
// specified, a “null” provider will be used which will result in all defaults
// being used.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,15 @@ message HttpConnectionManager {
repeated type.tracing.v2.CustomTag custom_tags = 8;

// Configuration for an external tracing provider.
// If not specified, Envoy will fall back to using tracing provider configuration
// from the bootstrap config.
// [#not-implemented-hide:]
// If not specified, no tracing will be performed.
//
// .. attention::
// Please be aware that *envoy.tracers.opencensus* provider can only be configured once
// in Envoy lifetime.
// Any attempts to reconfigure it or to use different configurations for different HCM filters
// will be rejected.
// Such a constraint is inherent to OpenCensus itself. It cannot be overcome without changes
// on OpenCensus side.
trace.v2.Tracing.Http provider = 9;
}

Expand Down
20 changes: 15 additions & 5 deletions api/envoy/config/trace/v2/trace.proto
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,22 @@ option (udpa.annotations.file_status).package_version_status = FROZEN;
// [#protodoc-title: Tracing]
// Tracing :ref:`architecture overview <arch_overview_tracing>`.

// The tracing configuration specifies global
// settings for the HTTP tracer used by Envoy. The configuration is defined by
// the :ref:`Bootstrap <envoy_api_msg_config.bootstrap.v2.Bootstrap>` :ref:`tracing
// <envoy_api_field_config.bootstrap.v2.Bootstrap.tracing>` field. Envoy may support other tracers
// in the future, but right now the HTTP tracer is the only one supported.
// The tracing configuration specifies settings for an HTTP tracer provider used by Envoy.
//
// Envoy may support other tracers in the future, but right now the HTTP tracer is the only one
// supported.
//
// .. attention::
//
// Use of this message type has been deprecated in favor of direct use of
// :ref:`Tracing.Http <envoy_api_msg_config.trace.v2.Tracing.Http>`.
message Tracing {
// Configuration for an HTTP tracer provider used by Envoy.
//
// The configuration is defined by the
// :ref:`HttpConnectionManager.Tracing <envoy_api_msg_config.filter.network.http_connection_manager.v2.HttpConnectionManager.Tracing>`
// :ref:`provider <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.Tracing.provider>`
// field.
message Http {
// The name of the HTTP trace driver to instantiate. The name must match a
// supported HTTP trace driver. Built-in trace drivers:
Expand Down
20 changes: 15 additions & 5 deletions api/envoy/config/trace/v3/trace.proto
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,24 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// [#protodoc-title: Tracing]
// Tracing :ref:`architecture overview <arch_overview_tracing>`.

// The tracing configuration specifies global
// settings for the HTTP tracer used by Envoy. The configuration is defined by
// the :ref:`Bootstrap <envoy_api_msg_config.bootstrap.v3.Bootstrap>` :ref:`tracing
// <envoy_api_field_config.bootstrap.v3.Bootstrap.tracing>` field. Envoy may support other tracers
// in the future, but right now the HTTP tracer is the only one supported.
// The tracing configuration specifies settings for an HTTP tracer provider used by Envoy.
//
// Envoy may support other tracers in the future, but right now the HTTP tracer is the only one
// supported.
//
// .. attention::
//
// Use of this message type has been deprecated in favor of direct use of
// :ref:`Tracing.Http <envoy_api_msg_config.trace.v3.Tracing.Http>`.
message Tracing {
Copy link
Member

Choose a reason for hiding this comment

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

Is this message actually used anymore or just the inner HTTP message? Should this message actually be tagged for deletion with the inner message moved somehow? I don't think the tool supports this but we might want to file an issue here? cc @htuch

Copy link
Member Author

Choose a reason for hiding this comment

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

config.trace.v3.Tracing will not be used anymore in v4.

I don't see a support for marking the entire message type as deprecated. The closest example is v2.Bootstrap.Runtime which only has (deprecated) comment in the docs.

I've added an "attention" message to config.trace.v3.Tracing for now.

@htuch Should I open an issue or you can phrase it better what needs to be done ?

Copy link
Member

Choose a reason for hiding this comment

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

Please open an issue to track this. We will need some tooling work I think to make sure this message gets deleted/moved/etc. I'm not sure the best way of handling.

Copy link
Member Author

Choose a reason for hiding this comment

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

opened #10740

option (udpa.annotations.versioning).previous_message_type = "envoy.config.trace.v2.Tracing";

// Configuration for an HTTP tracer provider used by Envoy.
//
// The configuration is defined by the
// :ref:`HttpConnectionManager.Tracing <envoy_api_msg_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing>`
// :ref:`provider <envoy_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.provider>`
// field.
message Http {
option (udpa.annotations.versioning).previous_message_type =
"envoy.config.trace.v2.Tracing.Http";
Expand Down
20 changes: 15 additions & 5 deletions api/envoy/config/trace/v4alpha/trace.proto
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,24 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO
// [#protodoc-title: Tracing]
// Tracing :ref:`architecture overview <arch_overview_tracing>`.

// The tracing configuration specifies global
// settings for the HTTP tracer used by Envoy. The configuration is defined by
// the :ref:`Bootstrap <envoy_api_msg_config.bootstrap.v4alpha.Bootstrap>` :ref:`tracing
// <envoy_api_field_config.bootstrap.v4alpha.Bootstrap.tracing>` field. Envoy may support other tracers
// in the future, but right now the HTTP tracer is the only one supported.
// The tracing configuration specifies settings for an HTTP tracer provider used by Envoy.
//
// Envoy may support other tracers in the future, but right now the HTTP tracer is the only one
// supported.
//
// .. attention::
//
// Use of this message type has been deprecated in favor of direct use of
// :ref:`Tracing.Http <envoy_api_msg_config.trace.v4alpha.Tracing.Http>`.
message Tracing {
option (udpa.annotations.versioning).previous_message_type = "envoy.config.trace.v3.Tracing";

// Configuration for an HTTP tracer provider used by Envoy.
//
// The configuration is defined by the
// :ref:`HttpConnectionManager.Tracing <envoy_api_msg_extensions.filters.network.http_connection_manager.v4alpha.HttpConnectionManager.Tracing>`
// :ref:`provider <envoy_api_field_extensions.filters.network.http_connection_manager.v4alpha.HttpConnectionManager.Tracing.provider>`
// field.
message Http {
option (udpa.annotations.versioning).previous_message_type =
"envoy.config.trace.v3.Tracing.Http";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,15 @@ message HttpConnectionManager {
repeated type.tracing.v3.CustomTag custom_tags = 8;

// Configuration for an external tracing provider.
// If not specified, Envoy will fall back to using tracing provider configuration
// from the bootstrap config.
// [#not-implemented-hide:]
// If not specified, no tracing will be performed.
//
// .. attention::
// Please be aware that *envoy.tracers.opencensus* provider can only be configured once
// in Envoy lifetime.
// Any attempts to reconfigure it or to use different configurations for different HCM filters
// will be rejected.
// Such a constraint is inherent to OpenCensus itself. It cannot be overcome without changes
// on OpenCensus side.
config.trace.v3.Tracing.Http provider = 9;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,15 @@ message HttpConnectionManager {
repeated type.tracing.v3.CustomTag custom_tags = 8;

// Configuration for an external tracing provider.
// If not specified, Envoy will fall back to using tracing provider configuration
// from the bootstrap config.
// [#not-implemented-hide:]
// If not specified, no tracing will be performed.
//
// .. attention::
// Please be aware that *envoy.tracers.opencensus* provider can only be configured once
// in Envoy lifetime.
// Any attempts to reconfigure it or to use different configurations for different HCM filters
// will be rejected.
// Such a constraint is inherent to OpenCensus itself. It cannot be overcome without changes
// on OpenCensus side.
config.trace.v4alpha.Tracing.Http provider = 9;
}

Expand Down
13 changes: 6 additions & 7 deletions configs/envoy_double_proxy_v2.template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@
typed_config: {}
tracing:
operation_name: INGRESS
provider:
name: envoy.tracers.lightstep
typed_config:
"@type": type.googleapis.com/envoy.config.trace.v2.LightstepConfig
access_token_file: "/etc/envoy/lightstep_access_token"
collector_cluster: lightstep_saas
common_http_protocol_options:
idle_timeout: 840s
access_log:
Expand Down Expand Up @@ -177,13 +183,6 @@ stats_sinks:
typed_config:
"@type": type.googleapis.com/envoy.config.metrics.v2.StatsdSink
tcp_cluster_name: statsd
tracing:
http:
name: envoy.tracers.lightstep
typed_config:
"@type": type.googleapis.com/envoy.config.trace.v2.LightstepConfig
access_token_file: "/etc/envoy/lightstep_access_token"
collector_cluster: lightstep_saas
layered_runtime:
layers:
- name: root
Expand Down
13 changes: 6 additions & 7 deletions configs/envoy_front_proxy_v2.template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@
add_user_agent: true
tracing:
operation_name: INGRESS
provider:
name: envoy.tracers.lightstep
typed_config:
"@type": type.googleapis.com/envoy.config.trace.v2.LightstepConfig
collector_cluster: lightstep_saas
access_token_file: "/etc/envoy/lightstep_access_token"
common_http_protocol_options:
idle_timeout: 840s
access_log:
Expand Down Expand Up @@ -155,13 +161,6 @@ cluster_manager:
outlier_detection:
event_log_path: /var/log/envoy/outlier_events.log
flags_path: /etc/envoy/flags
tracing:
http:
name: envoy.tracers.lightstep
typed_config:
"@type": type.googleapis.com/envoy.config.trace.v2.LightstepConfig
collector_cluster: lightstep_saas
access_token_file: "/etc/envoy/lightstep_access_token"
layered_runtime:
layers:
- name: root
Expand Down
7 changes: 0 additions & 7 deletions configs/envoy_service_to_service_v2.template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -551,13 +551,6 @@ stats_sinks:
"@type": type.googleapis.com/envoy.config.metrics.v2.StatsdSink
tcp_cluster_name: statsd
watchdog: {}
tracing:
http:
name: envoy.tracers.lightstep
typed_config:
"@type": type.googleapis.com/envoy.config.trace.v2.LightstepConfig
access_token_file: "/etc/envoy/lightstep_access_token"
collector_cluster: lightstep_saas
layered_runtime:
layers:
- name: root
Expand Down
3 changes: 3 additions & 0 deletions docs/root/intro/deprecated.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ Deprecated items below are listed in chronological order.

1.15.0 (Pending)
================
* Tracing provider configuration as part of :ref:`bootstrap config <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.tracing>`
has been deprecated in favor of configuration as part of :ref:`HTTP connection manager
<envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.provider>`.

1.14.0 (April 8, 2020)
======================
Expand Down
2 changes: 2 additions & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ Version history

1.15.0 (Pending)
================
* tracing: tracing configuration has been made fully dynamic and every HTTP connection manager
can now have a separate :ref:`tracing provider <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.provider>`.

1.14.0 (April 8, 2020)
======================
Expand Down
40 changes: 20 additions & 20 deletions examples/jaeger-native-tracing/front-envoy-jaeger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,26 @@ static_resources:
typed_config:
"@type": type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager
generate_request_id: true
tracing:
provider:
name: envoy.tracers.dynamic_ot
typed_config:
"@type": type.googleapis.com/envoy.config.trace.v2.DynamicOtConfig
library: /usr/local/lib/libjaegertracing_plugin.so
config:
service_name: front-proxy
sampler:
type: const
param: 1
reporter:
localAgentHostPort: jaeger:6831
headers:
jaegerDebugHeader: jaeger-debug-id
jaegerBaggageHeader: jaeger-baggage
traceBaggageHeaderPrefix: uberctx-
baggage_restrictions:
denyBaggageOnInitializationFailure: false
hostPort: ""
codec_type: auto
stat_prefix: ingress_http
route_config:
Expand Down Expand Up @@ -45,26 +65,6 @@ static_resources:
socket_address:
address: service1
port_value: 80
tracing:
http:
name: envoy.tracers.dynamic_ot
typed_config:
"@type": type.googleapis.com/envoy.config.trace.v2.DynamicOtConfig
library: /usr/local/lib/libjaegertracing_plugin.so
config:
service_name: front-proxy
sampler:
type: const
param: 1
reporter:
localAgentHostPort: jaeger:6831
headers:
jaegerDebugHeader: jaeger-debug-id
jaegerBaggageHeader: jaeger-baggage
traceBaggageHeaderPrefix: uberctx-
baggage_restrictions:
denyBaggageOnInitializationFailure: false
hostPort: ""
admin:
access_log_path: "/dev/null"
address:
Expand Down
Loading