Make TraceOptions optional. #12
Conversation
|
/cc @SergeyKanzhelev |
| 2. Bug in caller | ||
| 3. Different load between caller service and callee service might force callee | ||
| to down sample. | ||
| 1. Trust and abuse. |
There was a problem hiding this comment.
It was never clear to me what this means. Maybe a bit of an explanation?
There was a problem hiding this comment.
I believe this meant a bad actor can always say sampled for a ton of requests. If you don't trust the sampled bit, you can ignore it.
HTTP_HEADER_FORMAT.md
Outdated
| The behavior of other bits is currently undefined. | ||
| ##### Bits behavior definition (01234567): | ||
| * The least significant bit (the 7th bit) provides recommendation whether the request should be | ||
| traced or not (```1``` recommends the request should be traced, ```0``` means the caller does not |
There was a problem hiding this comment.
nit: curious why triple quoting things that aren't broken by newlines?
|
Why have trace options defined like this at all? I think the spec would be way better off by removing trace options from this header and pushing that functionality to a separate header that is more flexible. |
|
My commentary is that the sampled bit being optional was a good thing as
fits existing use in B3, making it a pragmatic replacement. If we remove
the sampled bit, we couldnt use it without a yet to be defined replacement
header. If the proposal swings the way of removing the trace options bit
(again.. because we have discussed this for a year) I would suggest against
it.
|
|
Fwiw I opened up #8 to track the discussion around sampling priority (which while sounding similar is different than trace option 1). I could open another issue to collect the historical discusson around the flag, if helpful. There is a lot of context in 1 bit and this single bit makes a relatively straightforward port of exisiting B3 code to this format. Adoptability of something that exists in dozens if not a hundred projects is fair game to retain imho. In our last workshop we realized people do need some incentive to change off B3 and making it easy is at least a lack of disincentive. |
|
Ok, that's a fair argument. Sorry I am pretty late to the game here. Still trying to get my context fully spun up. In that case, if the intent is for trace options to be extensible, we should clarify the process by which additional flags are proposed and ratified. |
heh |
|
I will merge this initially because this moves us towards #15 |
Based on the comments from the PR #1 and the issue #4 seems that not everybody uses the trace-options.
When the trace-options are missing every bit has to have a default value that is used. In general this will not be missing but in cases where users are concerned about extra bytes on wire not sending the trace-options is acceptable.