-
Notifications
You must be signed in to change notification settings - Fork 31
feat: ignore existing trace #99
Conversation
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
kikito
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @greut , thank you for this PR.
Do you think you could add some tests for it? See how other options are tested in the spec/ folder.
the handler is not tested so far; I can take a look into it but it requires some work. |
|
Not sure about this change, I would implement this as a request sampler
instead which is an abstraction that always unsample traces
https://github.com/openzipkin/zipkin-php/blob/master/src/Zipkin/Instrumentation/Http/Server/HttpServerTracing.php#L49.
This opens he possibility to a broader abstraction for different sampling
strategies based on the request.
Current implementation looks like mixing different concepts.
José Carlos Chávez
tir. 9. feb. 2021 kl. 18:54 skrev Yoan Blanc <[email protected]>:
… Do you think you could add some tests for it? See how other options are
tested in the spec/ folder.
the handler is not tested so far; I can take a look into it but it
requires some work.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#99 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXOYATDBJ4NDAGZGNNYQ3DS6FZG3ANCNFSM4VYSCO6A>
.
|
@jcchavezs Thanks, however I dont' quite understand how it works from the link you've provided. The goal is not to act on the sampling strategy but on the trace capture one. Kong being exposed to the internet, we don't want it to accept incoming traces. |
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
|
Thanks for this PR. I merged it with small modifications (including a test) in #125
That could be an option at the Zipkin level, but not at the Kong plugin level - that would require us to include source code as part of the plugin configuration, which is something we are not prepared to support for now. There are considerations about security and permissions to be had, and that would drag this discussion for even longer. For now, the presented feature is good enough. |
Using Kong as our API Gateway, we'd like to prevent external actor to pollute the traces. It's currently done using the request-transformer plugin (Kong/kong-plugin-request-transformer#32) but implies lowering the PRIORITY of this zipkin.
By setting
header_typetoignore, it won't read any tracing headers from the incoming request and start a new one using thedefault_header_typevalue (or falling back tob3)