Skip to content

tracing: Fix spankind = CLIENT bug when sharedSpanContext is false#13193

Merged
mattklein123 merged 1 commit intoenvoyproxy:masterfrom
objectiser:spankindbug
Sep 21, 2020
Merged

tracing: Fix spankind = CLIENT bug when sharedSpanContext is false#13193
mattklein123 merged 1 commit intoenvoyproxy:masterfrom
objectiser:spankindbug

Conversation

@objectiser
Copy link
Copy Markdown
Contributor

Commit Message:
Introducing the sharedSpanContext flag, to control whether shared or new span ids were used, inadvertently affected the value used for the span kind annotation. This PR separates out setting the annotation from the logic controlling whether span ids are shared or not.

Risk Level: low
Testing: Enhanced the unit tests.

Docs Changes: n/a
Release Notes:

Fixes #13141

Signed-off-by: Gary Brown <gary@brownuk.com>
@dio
Copy link
Copy Markdown
Member

dio commented Sep 21, 2020

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines, to retry CircleCI checks, use /retest-circle.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13193 (comment) was created by @dio.

see: more, trace.

Copy link
Copy Markdown
Member

@dio dio left a comment

Choose a reason for hiding this comment

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

Thanks, @objectiser! Most of my comments will be on how we put comments (as a complete sentence), but I have a plan to drop v1 and model this tracer to support v2 only, probably we can do that when the time comes 🙂.

SpanPtr parent_span = tracer.startSpan(config, "parent_span", timestamp);
SpanContext parent_context(*parent_span);

// An CS annotation must have been added
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.

nit: s/An/A.

@mattklein123 mattklein123 merged commit f4a800f into envoyproxy:master Sep 21, 2020
@objectiser objectiser deleted the spankindbug branch September 23, 2020 16:14
istio-testing pushed a commit to istio/envoy that referenced this pull request Sep 29, 2020
false (envoyproxy#13193)

Signed-off-by: Gary Brown <gary@brownuk.com>
Signed-off-by: Dmitri Dolguikh <dolguik@redhat.com>
istio-testing pushed a commit to istio/envoy that referenced this pull request Oct 2, 2020
false (envoyproxy#13193)

Signed-off-by: Gary Brown <gary@brownuk.com>
Signed-off-by: Dmitri Dolguikh <ddolguik@redhat.com>
istio-testing pushed a commit to istio/envoy that referenced this pull request Oct 5, 2020
false (envoyproxy#13193)

Signed-off-by: Gary Brown <gary@brownuk.com>
Signed-off-by: Dmitri Dolguikh <dolguik@redhat.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.

All spans get kind = CLIENT when sharedSpanContext = false

3 participants