Skip to content

Expose trace sampling controls in the public API #375

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.

Sign up for GitHub

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

Closed
hausdorff wants to merge 6 commits into from
Closed

Expose trace sampling controls in the public API #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2c4373d
Expose trace sampling controls in the public API
hausdorff Dec 29, 2017
33e1cea
Move the trace sampling controls to HttpConnectionManager
hausdorff Dec 30, 2017
2ebd52a
Use dedicated Percentage message type for percentages
hausdorff Dec 30, 2017
5f4defe
Small documentation fixes
hausdorff Dec 30, 2017
6938a37
Docs fixes
hausdorff Dec 30, 2017
bec964a
Update documentation and percent data types
hausdorff Jan 3, 2018
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
8 changes: 8 additions & 0 deletions api/base.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,3 +227,11 @@ message TransportSocket {
// See the supported transport socket implementations for further documentation.
google.protobuf.Struct config = 2;
}

// A percent, rounded down to the nearest 0.01%. Typically used to specify things like target
// sampling percentages among tracing requests (as in, e.g., :ref:`HTTP Connection Manager tracing
// <envoy_api_field_filter.network.HttpConnectionManager.tracing>`).
message HundredthsRoundedPercent {
// The percent, a float between 0 and 100. Rounded to the nearest 0.01%.
double value = 1 [(validate.rules).double = {gte: 0, lte: 100}];
}
17 changes: 17 additions & 0 deletions api/filter/network/http_connection_manager.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,23 @@ message HttpConnectionManager {
// populate the tag name, and the header value is used to populate the tag value. The tag is
// created if the specified header name is present in the request's headers.
repeated string request_headers_for_tags = 2;

// Target percentage of requests managed by this HTTP connectionmanager that will be force
// traced if the *x-client-trace-id* header is set. Defaults to 100%. This variable is a direct
// analog for the variable of the same name in the :ref:`HTTP Connection Manager
// <config_http_conn_man_runtime>`.
double client_sampling = 3 [(validate.rules).double = {gte: 0, lte: 100}];
Copy link
Member

@htuch htuch Jan 3, 2018

Choose a reason for hiding this comment

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

I think we should do the following:

  • Add a universal Percent message, which enforced [0..100] range and is backed by double.
  • Express locally, at each field, what the actual effective interpretation will be. This interpretation might change over time (e.g. it might be rounded to nearest 1% today, in later Envoy releases it might be 0.1%). I agree with @wora that folks supplying the API data should be able to express their intents as fine grained as they wish, but at the same time, users need to know what the limits at the implementation level are.


// Target percentage of requests managed by this HTTP connection manager that will be traced
// after all other checks have been applied (force tracing, sampling, etc.). Defaults to 100%.
// This variable is a direct analog for the variable of the same name in the :ref:`HTTP
// Connection Manager <config_http_conn_man_runtime>`.
double sampling = 4 [(validate.rules).double = {gte: 0, lte: 100}];
Copy link
Member

@htuch htuch Jan 3, 2018

Choose a reason for hiding this comment

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

Can you start a STYLE.md in root with a single bullet point for now, which captures the point that @wora made about preferring double over float for JSON conversion reasons? This is is something we should now start to be more consistent about, and it's one of these missing proto3 style guidelines that would be good to codify. Thanks!


// Target percentage of requests managed by this HTTP connection manager that will be randomly
// traced. Defaults to 100%. This variable is a direct analog for the variable of the same name
// in the :ref:`HTTP Connection Manager <config_http_conn_man_runtime>`.
HundredthsRoundedPercent random_sampling = 5;
}

// Presence of the object defines whether the connection manager
Expand Down