Tracing: revamp extract/inject span context#444
Conversation
|
|
||
| if (request_headers.OtSpanContext()) { | ||
| // Extract downstream context from HTTP carrier. | ||
| std::string parent_context = Base64::decode(request_headers.OtSpanContext()->value().c_str()); |
There was a problem hiding this comment.
What happens if the header is invalid or ParseFromString below fails? Do we handle that?
There was a problem hiding this comment.
if header is invalid CarrierStruct will not be populated properly and we'll get spans joined in a single trace by x-request-id, not a big deal
There was a problem hiding this comment.
Can you add comment that this code is safe if Base64::decode returns empty string or the data is invalid.
There was a problem hiding this comment.
ParseFromString in the worst case will leave CarrierStruct empty, that'll not crash or anything
There was a problem hiding this comment.
i'll add comment.
| lightstep::envoy::CarrierStruct ctx; | ||
| tracer.Inject(active_span->context(), lightstep::CarrierFormat::EnvoyProtoCarrier, | ||
| lightstep::envoy::ProtoWriter(&ctx)); | ||
| Buffer::OwnedImpl buffer(ctx.SerializeAsString()); |
There was a problem hiding this comment.
Instead of copying this just to call the function, can you have a version of Base64::encode that just takes memory and length directly.
There was a problem hiding this comment.
let me look here, should be doable but will schedule encode change on a separate PR first than.
There was a problem hiding this comment.
updated, will post update on this PR soon
* remove private repo/branches follow up on envoyproxy#404 missing (ah!) code review updates * review comments/reverting unecessary changes
|
Curious: what drove the change from 256 -> 128? This is causing issues with some users now and I'd like to understand the reasoning behind the change. |
Update to latest master.
…k_filter.rst (envoyproxy#444) * Update health_check_filter.rst * Update health_check_filter.rst has some translate bug
This is required for us to update to Xcode 11 Signed-off-by: Keith Smiley <keithbsmiley@gmail.com> Signed-off-by: JP Simard <jp@jpsim.com>
This is required for us to update to Xcode 11 Signed-off-by: Keith Smiley <keithbsmiley@gmail.com> Signed-off-by: JP Simard <jp@jpsim.com>
3 participants
Mostly this change is the same as 21dbbaf
It's different in: