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

Spec compliance for in-memory representation of span and trace IDs #232

Closed
fbogsany opened this issue May 7, 2020 · 0 comments · Fixed by #280
Closed

Spec compliance for in-memory representation of span and trace IDs #232

fbogsany opened this issue May 7, 2020 · 0 comments · Fixed by #280
Assignees
Milestone

Comments

@fbogsany
Copy link
Contributor

fbogsany commented May 7, 2020

The specification for SpanContext requires that:

TraceId A valid trace identifier is a 16-byte array with at least one non-zero byte.

SpanId A valid span identifier is an 8-byte array with at least one non-zero byte.

Our representation is 16 and 32 character hex strings:

module Trace
# An invalid trace identifier, a 16-byte array with all zero bytes, encoded
# as a hexadecimal string.
INVALID_TRACE_ID = ('0' * 32).freeze
# An invalid span identifier, an 8-byte array with all zero bytes, encoded
# as a hexadecimal string.
INVALID_SPAN_ID = ('0' * 16).freeze
# Generates a valid trace identifier, a 16-byte array with at least one
# non-zero byte, encoded as a hexadecimal string.
#
# @return [String] a hexadecimal string encoding of a valid trace ID.
def self.generate_trace_id
loop do
id = Random::DEFAULT.bytes(16).unpack1('H*')
return id unless id == INVALID_TRACE_ID
end
end
# Generates a valid span identifier, an 8-byte array with at least one
# non-zero byte, encoded as a hexadecimal string.
#
# @return [String] a hexadecimal string encoding of a valid span ID.
def self.generate_span_id
loop do
id = Random::DEFAULT.bytes(8).unpack1('H*')
return id unless id == INVALID_SPAN_ID
end
end
end
end

We should conform to the spec. This actually makes a few things better, including fewer allocations overall and making translation to things that don't use hex strings much easier to understand and prove correct.

@mwear mwear added this to the Alpha v0.5 milestone Jun 4, 2020
@fbogsany fbogsany self-assigned this Jun 4, 2020
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 a pull request may close this issue.

2 participants