Expose trace sampling controls#444
Conversation
Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
|
|
||
| // 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>`. |
There was a problem hiding this comment.
I think it would be useful to add a note comparing the two fields. It's not entirely obvious to me how they differ.
There was a problem hiding this comment.
I was similarly confused. I attempted a bit of clarification based on my read of the existing documentation and code. Hopefully my attempt has added clarity. Please let me know what you think.
|
FYI this PR was just closed that was working on the same thing. Please reconcile with the giant set of comments in there. Thank you. #375 |
|
Oops sorry didn't see comment on other PR. Yes if this is reconciled with that, would love to see this move forward. |
|
@mattklein123 as far as I am aware, this is reconciled -- with the new bits being a move to use the newly-added |
|
|
||
| // 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 named 'global_enabled' in the :ref:`HTTP |
| Percent sampling = 3; | ||
|
|
||
| // 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 |
|
|
||
| // 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>`. |
Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
|
PTAL. I've tried to clean up the docs as suggested in comments. I also added back the I've renamed |
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the doc explanations, very helpful (and I believe accurate). Just one small x-link nit.
| repeated string request_headers_for_tags = 2; | ||
|
|
||
| // Target percentage of requests managed by this HTTP connection manager that will be force | ||
| // traced if the *x-client-trace-id* header is set. This field is a direct analog for the |
There was a problem hiding this comment.
kyessenov
left a comment
kyessenov
left a comment
There was a problem hiding this comment.
Thank you for writing the explanation. It is very helpful.
|
Thanks for the detailed documentation. /lgtm |
htuch
left a comment
htuch
left a comment
There was a problem hiding this comment.
LGTM modulo remaining comments.
Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
douglas-reid
force-pushed
the
trace-sampling
branch
from
174e189 to
9aa7799
Compare
|
Thanks for the quick reviews! I've added the ref for x-client-trace-id as requested. PTAL. |
5 participants
This PR adds trace sampling controls to the public API. It is a continuation of work from PR #375.
The controls added:
Tracing.sampling(equivalent to tracing.global_enabled)Tracing.random_sampling(equivalent to tracing.random_sampling)Signed-off-by: Douglas Reid douglas-reid@users.noreply.github.com