Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: rename SpanContext to SpanReference #440

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/benchmarks/tracer_bench.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

links = Array.new(3) do
OpenTelemetry::Trace::Link.new(
OpenTelemetry::Trace::SpanContext.new,
OpenTelemetry::Trace::SpanReference.new,
attributes
)
end
Expand Down
2 changes: 1 addition & 1 deletion api/lib/opentelemetry/trace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def with_span(span)

require 'opentelemetry/trace/link'
require 'opentelemetry/trace/trace_flags'
require 'opentelemetry/trace/span_context'
require 'opentelemetry/trace/span_reference'
require 'opentelemetry/trace/span_kind'
require 'opentelemetry/trace/span'
require 'opentelemetry/trace/status'
Expand Down
14 changes: 7 additions & 7 deletions api/lib/opentelemetry/trace/link.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ class Link

private_constant :EMPTY_ATTRIBUTES

# Returns the {SpanContext} for this link
# Returns the {SpanReference} for this link
#
# @return [SpanContext]
attr_reader :context
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like this should be span_context, although we avoid that name in Span due to stuttering (but perhaps we shouldn't?).

# @return [SpanReference]
attr_reader :reference

# Returns the frozen attributes for this link.
#
Expand All @@ -27,19 +27,19 @@ class Link

# Returns a new immutable {Link}.
#
# @param [SpanContext] span_context The context of the linked {Span}.
# @param [SpanReference] span_reference The reference to the linked {Span}.
# @param [optional Hash{String => String, Numeric, Boolean, Array<String, Numeric, Boolean>}]
# attributes A hash of attributes for this link. Attributes will be
# frozen during Link initialization.
# @return [Link]
def initialize(span_context, attributes = nil)
@context = span_context
def initialize(span_reference, attributes = nil)
@reference = span_reference
@attributes = attributes.freeze || EMPTY_ATTRIBUTES
end

# Returns true if two {Link}s are equal.
def ==(other)
other.context == @context && other.attributes == @attributes
other.reference == @reference && other.attributes == @attributes
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ def initialize(traceparent_key: 'traceparent',
@tracestate_key = tracestate_key
end

# Extract a remote {Trace::SpanContext} from the supplied carrier.
# Invalid headers will result in a new, valid, non-remote {Trace::SpanContext}.
# Extract a remote {Trace::SpanReference} from the supplied carrier.
# Invalid headers will result in a new, valid, non-remote {Trace::SpanReference}.
#
# @param [Carrier] carrier The carrier to get the header from.
# @param [Context] context The context to be updated with extracted context
Expand All @@ -42,12 +42,12 @@ def extract(carrier, context, &getter)

tracestate = getter.call(carrier, @tracestate_key)

span_context = Trace::SpanContext.new(trace_id: tp.trace_id,
span_id: tp.span_id,
trace_flags: tp.flags,
tracestate: tracestate,
remote: true)
span = Trace::Span.new(span_context: span_context)
span_reference = Trace::SpanReference.new(trace_id: tp.trace_id,
span_id: tp.span_id,
trace_flags: tp.flags,
tracestate: tracestate,
remote: true)
span = Trace::Span.new(span_reference: span_reference)
OpenTelemetry::Trace.context_with_span(span)
rescue OpenTelemetry::Error
context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,19 @@ def initialize(traceparent_key: 'traceparent',
# carrier, header key, header value to the setter.
# @return [Object] the carrier with context injected
def inject(carrier, context, &setter)
return carrier unless (span_context = span_context_from(context))
return carrier unless (span_reference = span_reference_from(context)).valid?

setter ||= DEFAULT_SETTER
setter.call(carrier, @traceparent_key, TraceParent.from_context(span_context).to_s)
setter.call(carrier, @tracestate_key, span_context.tracestate) unless span_context.tracestate.nil?
setter.call(carrier, @traceparent_key, TraceParent.from_span_reference(span_reference).to_s)
setter.call(carrier, @tracestate_key, span_reference.tracestate) unless span_reference.tracestate.nil?

carrier
end

private

def span_context_from(context)
OpenTelemetry::Trace.current_span(context)&.context
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe we can get rid of the lonely operator here.

def span_reference_from(context)
OpenTelemetry::Trace.current_span(context).reference
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module Propagation
module TraceContext
# A TraceParent is an implementation of the W3C trace context specification
# https://www.w3.org/TR/trace-context/
# {Trace::SpanContext}
# {Trace::SpanReference}
class TraceParent
InvalidFormatError = Class.new(Error)
InvalidVersionError = Class.new(Error)
Expand All @@ -25,16 +25,16 @@ class TraceParent
REGEXP = /^(?<version>[A-Fa-f0-9]{2})-(?<trace_id>[A-Fa-f0-9]{32})-(?<span_id>[A-Fa-f0-9]{16})-(?<flags>[A-Fa-f0-9]{2})(?<ignored>-.*)?$/.freeze
private_constant :REGEXP

INVALID_TRACE_ID = OpenTelemetry::Trace::SpanContext::INVALID.hex_trace_id
INVALID_SPAN_ID = OpenTelemetry::Trace::SpanContext::INVALID.hex_span_id
INVALID_TRACE_ID = OpenTelemetry::Trace::SpanReference::INVALID.hex_trace_id
INVALID_SPAN_ID = OpenTelemetry::Trace::SpanReference::INVALID.hex_span_id
private_constant :INVALID_TRACE_ID, :INVALID_SPAN_ID

class << self
# Creates a new {TraceParent} from a supplied {Trace::SpanContext}
# @param [SpanContext] ctx The context
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should read span context

# Creates a new {TraceParent} from a supplied {Trace::SpanReference}
# @param [SpanReference] reference The span reference
# @return [TraceParent] a trace parent
def from_context(ctx)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps rename this from_span_context

new(trace_id: ctx.trace_id, span_id: ctx.span_id, flags: ctx.trace_flags)
def from_span_reference(reference)
new(trace_id: reference.trace_id, span_id: reference.span_id, flags: reference.trace_flags)
end

# Deserializes the {TraceParent} from the string representation
Expand Down
12 changes: 6 additions & 6 deletions api/lib/opentelemetry/trace/span.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ module Trace
#
# {Span} must be ended by calling {#finish}.
class Span
# Retrieve the spans SpanContext
# Retrieve the spans SpanReference
#
# The returned value may be used even after the Span is finished.
#
# @return [SpanContext]
attr_reader :context
# @return [SpanReference]
attr_reader :reference

# Spans must be created using {Tracer}. This is for internal use only.
#
# @api private
def initialize(span_context: nil)
@context = span_context || SpanContext.new
def initialize(span_reference: nil)
@reference = span_reference || SpanReference.new
end

# Return whether this span is recording.
Expand Down Expand Up @@ -131,7 +131,7 @@ def finish(end_timestamp: nil)
self
end

INVALID = new(span_context: SpanContext::INVALID)
INVALID = new(span_reference: SpanReference::INVALID)
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,21 @@

module OpenTelemetry
module Trace
# A SpanContext contains the state that must propagate to child {Span}s and across process boundaries.
# A SpanReference contains the state that must propagate to child {Span}s and across process boundaries.
# It contains the identifiers (a trace ID and span ID) associated with the {Span}, a set of
# {TraceFlags}, a system-specific tracestate, and a boolean indicating that the SpanContext was
# {TraceFlags}, a system-specific tracestate, and a boolean indicating that the SpanReference was
# extracted from the wire.
class SpanContext
class SpanReference
attr_reader :trace_flags, :tracestate

# Returns a new {SpanContext}.
# Returns a new {SpanReference}.
#
# @param [optional String] trace_id The trace ID associated with a {Span}.
# @param [optional String] span_id The span ID associated with a {Span}.
# @param [optional TraceFlags] trace_flags The trace flags associated with a {Span}.
# @param [optional String] tracestate The tracestate associated with a {Span}. May be nil.
# @param [optional Boolean] remote Whether the {SpanContext} was extracted from the wire.
# @return [SpanContext]
# @param [optional Boolean] remote Whether the {SpanReference} was extracted from the wire.
# @return [SpanReference]
def initialize(
trace_id: Trace.generate_trace_id,
span_id: Trace.generate_span_id,
Expand Down Expand Up @@ -59,21 +59,21 @@ def hex_span_id
# @return [String] An 8-byte binary string.
attr_reader :span_id

# Returns true if the {SpanContext} has a non-zero trace ID and non-zero span ID.
# Returns true if the {SpanReference} has a non-zero trace ID and non-zero span ID.
#
# @return [Boolean]
def valid?
@trace_id != INVALID_TRACE_ID && @span_id != INVALID_SPAN_ID
end

# Returns true if the {SpanContext} was propagated from a remote parent.
# Returns true if the {SpanReference} was propagated from a remote parent.
#
# @return [Boolean]
def remote?
@remote
end

# Represents an invalid {SpanContext}, with an invalid trace ID and an invalid span ID.
# Represents an invalid {SpanReference}, with an invalid trace ID and an invalid span ID.
INVALID = new(trace_id: INVALID_TRACE_ID, span_id: INVALID_SPAN_ID)
end
end
Expand Down
6 changes: 3 additions & 3 deletions api/lib/opentelemetry/trace/tracer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ def start_root_span(name, attributes: nil, links: nil, start_timestamp: nil, kin
#
# @return [Span]
def start_span(name, with_parent: nil, attributes: nil, links: nil, start_timestamp: nil, kind: nil)
span_context = OpenTelemetry::Trace.current_span(with_parent).context
span_reference = OpenTelemetry::Trace.current_span(with_parent).reference

if span_context.valid?
Span.new(span_context: span_context)
if span_reference.valid?
Span.new(span_reference: span_reference)
else
Span.new
end
Expand Down
20 changes: 10 additions & 10 deletions api/test/opentelemetry/trace/link_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,35 @@

describe OpenTelemetry::Trace::Link do
Link = OpenTelemetry::Trace::Link
let(:span_context) { OpenTelemetry::Trace::SpanContext.new }
let(:span_reference) { OpenTelemetry::Trace::SpanReference.new }
describe '.new' do
it 'accepts a span_context' do
link = Link.new(span_context)
_(link.context).must_equal(span_context)
it 'accepts a span_reference' do
link = Link.new(span_reference)
_(link.reference).must_equal(span_reference)
end

it 'returns a link with the given span context and attributes' do
link = Link.new(span_context, '1' => 1)
it 'returns a link with the given span reference and attributes' do
link = Link.new(span_reference, '1' => 1)
_(link.attributes).must_equal('1' => 1)
_(link.context).must_equal(span_context)
_(link.reference).must_equal(span_reference)
end

it 'returns a link with no attributes by default' do
link = Link.new(span_context)
link = Link.new(span_reference)
_(link.attributes).must_equal({})
end

it 'allows array-valued attributes' do
attributes = { 'foo' => [1, 2, 3] }
link = Link.new(span_context, attributes)
link = Link.new(span_reference, attributes)
_(link.attributes).must_equal(attributes)
end
end

describe '.attributes' do
it 'returns and freezes attributes passed in' do
attributes = { 'foo' => 'bar', 'bar' => 'baz' }
link = Link.new(span_context, attributes)
link = Link.new(span_reference, attributes)
_(link.attributes).must_equal(attributes)
_(link.attributes).must_be(:frozen?)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,24 +41,24 @@
_(yielded_keys.sort).must_equal([traceparent_key, tracestate_key])
end

it 'returns a remote SpanContext with fields from the traceparent and tracestate headers' do
it 'returns a remote SpanReference with fields from the traceparent and tracestate headers' do
ctx = extractor.extract(carrier, context) { |c, k| c[k] }
span_context = OpenTelemetry::Trace.current_span(ctx).context
_(span_context).must_be :remote?
_(span_context.trace_id).must_equal(("\0" * 15 + "\xaa").b)
_(span_context.span_id).must_equal(("\0" * 7 + "\xea").b)
_(span_context.trace_flags).must_be :sampled?
_(span_context.tracestate).must_equal('vendorname=opaquevalue')
span_reference = OpenTelemetry::Trace.current_span(ctx).reference
_(span_reference).must_be :remote?
_(span_reference.trace_id).must_equal(("\0" * 15 + "\xaa").b)
_(span_reference.span_id).must_equal(("\0" * 7 + "\xea").b)
_(span_reference.trace_flags).must_be :sampled?
_(span_reference.tracestate).must_equal('vendorname=opaquevalue')
end

it 'uses a default getter if one is not provided' do
ctx = extractor.extract(carrier, context)
span_context = OpenTelemetry::Trace.current_span(ctx).context
_(span_context).must_be :remote?
_(span_context.trace_id).must_equal(("\0" * 15 + "\xaa").b)
_(span_context.span_id).must_equal(("\0" * 7 + "\xea").b)
_(span_context.trace_flags).must_be :sampled?
_(span_context.tracestate).must_equal('vendorname=opaquevalue')
span_reference = OpenTelemetry::Trace.current_span(ctx).reference
_(span_reference).must_be :remote?
_(span_reference.trace_id).must_equal(("\0" * 15 + "\xaa").b)
_(span_reference.span_id).must_equal(("\0" * 7 + "\xea").b)
_(span_reference.trace_flags).must_be :sampled?
_(span_reference.tracestate).must_equal('vendorname=opaquevalue')
end

it 'returns original context on error' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

describe OpenTelemetry::Trace::Propagation::TraceContext::TextMapInjector do
Span = OpenTelemetry::Trace::Span
SpanContext = OpenTelemetry::Trace::SpanContext
SpanReference = OpenTelemetry::Trace::SpanReference

let(:traceparent_key) { 'traceparent' }
let(:tracestate_key) { 'tracestate' }
Expand All @@ -26,14 +26,14 @@
end
let(:tracestate_header) { 'vendorname=opaquevalue' }
let(:context) do
span_context = SpanContext.new(trace_id: ("\xff" * 16).b, span_id: ("\x11" * 8).b)
span = Span.new(span_context: span_context)
span_reference = SpanReference.new(trace_id: ("\xff" * 16).b, span_id: ("\x11" * 8).b)
span = Span.new(span_reference: span_reference)
OpenTelemetry::Trace.context_with_span(span, parent_context: Context.empty)
end
let(:context_with_tracestate) do
span_context = SpanContext.new(trace_id: ("\xff" * 16).b, span_id: ("\x11" * 8).b,
tracestate: tracestate_header)
span = Span.new(span_context: span_context)
span_reference = SpanReference.new(trace_id: ("\xff" * 16).b, span_id: ("\x11" * 8).b,
tracestate: tracestate_header)
span = Span.new(span_reference: span_reference)
OpenTelemetry::Trace.context_with_span(span, parent_context: Context.empty)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
Trace = OpenTelemetry::Trace
let(:good) do
flags = Trace::TraceFlags.from_byte(1)
TraceParent.from_context(Trace::SpanContext.new(trace_flags: flags))
TraceParent.from_span_reference(Trace::SpanReference.new(trace_flags: flags))
end

describe '.to_s' do
Expand All @@ -25,8 +25,8 @@
end

it 'should make a traceparent from a span context' do
sp = OpenTelemetry::Trace::SpanContext.new
tp = TraceParent.from_context(sp)
sp = OpenTelemetry::Trace::SpanReference.new
tp = TraceParent.from_span_reference(sp)

_(sp.trace_id).must_equal tp.trace_id
_(sp.span_id).must_equal tp.span_id
Expand Down
Loading