Skip to content

Set ids for spans#290

Merged
reyang merged 12 commits into
open-telemetry:masterfrom
nadiaciobanu:setids
Aug 24, 2020
Merged

Set ids for spans#290
reyang merged 12 commits into
open-telemetry:masterfrom
nadiaciobanu:setids

Conversation

@nadiaciobanu
Copy link
Copy Markdown
Contributor

Set trace_id, span_id and parent_span_id for spans using a random buffer generator.

Currently, the Span class does not support extracting the span context from a span (this functionality will be added in #143). For now, we create an invalid parent span context in the tracer and pass it to the span constructor. Since the parent span context is invalid, Span always generates new ids (rather than setting trace_id and parent_span_id based on the parent context).

Once Span supports extracting the span context, the parent span context will be extracted in the tracer, and passed into the Span constructor. The constructor will create a span context for the new span and set the ids in that context using the parent span context. Here is the issue for tracking this work: #285.

This change requires adding support for trace_id and span_id in SpanContext. Since SpanContext is not fully implemented yet (implemented in #143), trace_id and span_id are set to invalid (all zeros).

@nadiaciobanu nadiaciobanu requested a review from a team August 20, 2020 23:54
Comment thread api/test/trace/span_context_test.cc Outdated
Comment thread sdk/test/trace/tracer_test.cc Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 21, 2020

Codecov Report

Merging #290 into master will decrease coverage by 0.00%.
The diff coverage is 93.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #290      +/-   ##
==========================================
- Coverage   93.94%   93.93%   -0.01%     
==========================================
  Files         146      146              
  Lines        6538     6564      +26     
==========================================
+ Hits         6142     6166      +24     
- Misses        396      398       +2     
Impacted Files Coverage Δ
sdk/src/trace/span.h 0.00% <ø> (ø)
sdk/src/trace/span.cc 80.76% <85.71%> (+0.17%) ⬆️
api/include/opentelemetry/trace/span_context.h 100.00% <100.00%> (ø)
api/test/trace/span_context_test.cc 100.00% <100.00%> (ø)
sdk/src/trace/tracer.cc 68.96% <100.00%> (+1.10%) ⬆️
sdk/test/trace/tracer_test.cc 99.07% <100.00%> (+0.02%) ⬆️

Comment thread sdk/src/trace/span.cc Outdated
Comment thread sdk/src/trace/tracer.cc
Comment thread sdk/src/trace/tracer.cc
Comment thread sdk/src/trace/tracer.cc
Copy link
Copy Markdown

@IlyaKobelevskiy IlyaKobelevskiy left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@reyang reyang left a comment

Choose a reason for hiding this comment

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

LGTM.

@reyang reyang merged commit c694915 into open-telemetry:master Aug 24, 2020
@ZiweiZhao
Copy link
Copy Markdown

LGTM

@nadiaciobanu nadiaciobanu deleted the setids branch August 25, 2020 22:45
GerHobbelt pushed a commit to GerHobbelt/opentelemetry-cpp that referenced this pull request Jun 17, 2025
…ld-push-action-6.x

chore(deps): update docker/build-push-action action to v6.17.0
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.

6 participants