Skip to content

Tracing: Allow extract span context data from request headers and inject new span context into it#388

Merged
RomanDzhabarov merged 7 commits intomasterfrom
inject_extract_parent
Jan 30, 2017
Merged

Tracing: Allow extract span context data from request headers and inject new span context into it#388
RomanDzhabarov merged 7 commits intomasterfrom
inject_extract_parent

Conversation

@RomanDzhabarov
Copy link
Copy Markdown
Member

No description provided.

HEADER_FUNC(Upgrade) \
HEADER_FUNC(UserAgent)
HEADER_FUNC(UserAgent) \
HEADER_FUNC(OtSpanContext)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

alpha order

const LowerCaseString TransferEncoding{"transfer-encoding"};
const LowerCaseString Upgrade{"upgrade"};
const LowerCaseString UserAgent{"user-agent"};
const LowerCaseString OtSpanContext{"x-ot-span-context"};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

alpha order

class LightStepSpan : public Span {
public:
LightStepSpan(lightstep::Span& span);
LightStepSpan(lightstep::Span span);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

?

/**
* Inject span context into HTTP carrier.
*/
virtual void inject(Span* active_span, Http::HeaderMap& headers) PURE;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand why this needs to exist here. You only ever call inject() right after you call startSpan(), so just do it all inside the startSpan() flow of calls.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah, prob no compelling reason to extract that into separate method

LightStepSpanPtr active_span;

// Extract downstream context from HTTP carrier.
const std::string parent_context =
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

storing to variable here is not needed, just check if header exists, then pass c_str() directly to ParseFromString. (In the future it would be nice to avoid string conversion).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed

@RomanDzhabarov RomanDzhabarov merged commit 21dbbaf into master Jan 30, 2017
@mattklein123 mattklein123 deleted the inject_extract_parent branch January 30, 2017 23:18
RomanDzhabarov added a commit that referenced this pull request Feb 1, 2017
… and inject new span context into it (#388)"

This reverts commit 21dbbaf.
mattklein123 pushed a commit that referenced this pull request Feb 1, 2017
… and inject new span context into it (#388)" (#403)

This reverts commit 21dbbaf.
RomanDzhabarov added a commit that referenced this pull request Feb 8, 2017
… headers and inject new span context into it (#388)" (#403)"

This reverts commit 435cd65.
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
* Remove api_manager code.

* remove transcoding_repoistories
wolfguoliang pushed a commit to wolfguoliang/envoy that referenced this pull request Jan 23, 2021
…trap

zh-translation:docs/root/configuration/overview/bootstrap.rst
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Description: taking in the tip of master
Risk Level: low
Testing: CI and local for iOS

Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Description: taking in the tip of master
Risk Level: low
Testing: CI and local for iOS

Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
mathetake pushed a commit that referenced this pull request Mar 3, 2026
**Commit Message**

Ensure hover styling has same border-radius, so the buttons maintain the
shape when hovering.

Signed-off-by: Erica Hughberg <erica.sundberg.90@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants