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

Update backend to use Traces 0.14.1 #2

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
20 changes: 13 additions & 7 deletions lib/traces/backend/open_telemetry/interface.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module OpenTelemetry
TRACER = ::OpenTelemetry.tracer_provider.tracer(Traces::Backend::OpenTelemetry.name, Traces::Backend::OpenTelemetry::VERSION)

module Interface
def trace(name, attributes: nil, &block)
def trace(name, attributes: {}, &block)
Copy link
Author

Choose a reason for hiding this comment

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

attributes calls transform_keys below, which nil will not respond to.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that makes sense, alternatively we could write attributes&.transform_keys. Not sure what makes more sense.

span = TRACER.start_span(name, attributes: attributes.transform_keys(&:to_s))

begin
Expand All @@ -27,35 +27,41 @@ def trace(name, attributes: nil, &block)
end
rescue Exception => error
span&.record_exception(error)
span&.status = ::OpenTelemetry::Traces::Status.error("Unhandled exception of type: #{error.class}")
span&.status = ::OpenTelemetry::Trace::Status.error("Unhandled exception of type: #{error.class}")
Copy link
Author

Choose a reason for hiding this comment

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

appears to be a namespace change within OpenTelemetry

raise
ensure
span&.finish
end
end

def trace_context=(context)
span_context = ::OpenTelemetry::Traces::SpanContext.new(
span_context = ::OpenTelemetry::Trace::SpanContext.new(
trace_id: context.trace_id,
span_id: context.parent_id,
trace_flags: ::OpenTelemetry::Traces::TracesFlags.from_byte(context.flags),
trace_flags: ::OpenTelemetry::Trace::TraceFlags.from_byte(context.flags),
tracestate: context.state,
remote: context.remote?
)

span = ::OpenTelemetry::Trace.non_recording_span(span_context)

# the span seems to be updated correctly, and evaluating line 49 seems to propagate the trace/span ids
# but its not updated in my span
return ::OpenTelemetry::Trace.context_with_span(span)
end

def trace_context(span = ::OpenTelemetry::Trace.current_span)
# state = baggage.values(context: span.context)
if span_context = span.context
state = baggage.values(context: span.context)
Copy link
Author

Choose a reason for hiding this comment

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

state was an unused variable and the baggage api has changed, if this is important I need to understand the otel baggage concept and implement.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's important. The goal is to create a context which can be sent over the wire, e.g. over an HTTP request or encoded into a database query comment etc.

flags = 0

if span_context.trace_flags.sampled?
flags |= Context::SAMPLED
end
Comment on lines +55 to +59
Copy link
Author

Choose a reason for hiding this comment

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

I pulled this from the Datadog Backend. as it was implemented, the trace flags were expected to be provided as an integer, not an object.


return Context.new(
span_context.trace_id,
span_context.span_id,
span_context.trace_flags,
flags,
span_context.tracestate,
remote: span_context.remote?
)
Expand Down
54 changes: 52 additions & 2 deletions test/traces/backend/open_telemetry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,68 @@ def my_method(argument)
def my_method(argument)
Traces.trace("my_method", attributes: {argument: argument}) {super}
end

def my_span
Traces.trace('my_span') {|span| return span}
end

def my_context
Traces.trace('my_context') {|span| return Traces.trace_context}
end

def my_span_and_context
Traces.trace('my_span_and_context') {|span| return span, Traces.trace_context}
end
end

describe Traces::Backend::OpenTelemetry do
let(:instance) { MyClass.new }

it "has a version number" do
expect(Traces::Backend::OpenTelemetry::VERSION).not.to be == nil
end

it "can invoke trace wrapper" do
instance = MyClass.new

expect(Traces::Backend::OpenTelemetry::TRACER).to receive(:start_span)

instance.my_method(10)
end

describe OpenTelemetry::Trace::Span do
it "can yield an open telemetry span" do
expect(instance.my_span).to be_a(OpenTelemetry::Trace::Span)
end
end

describe "span and context" do
let(:span_and_context) {instance.my_span_and_context}
let(:span) {span_and_context.first}
let(:context) {span_and_context.last}
let(:new_context) {
Traces::Context.new(
SecureRandom.uuid,
SecureRandom.uuid,
0,
nil,
remote: false
)
}

it "can provide a trace context with a trace_id" do
expect(context).to have_attributes(trace_id: be != nil)
expect(span.context.trace_id).to be == context.trace_id
end

describe "#trace_context=" do
it "can update the trace context" do
expect(Traces.trace_context).to be != new_context

Traces.trace_context = new_context
expect(instance.my_span.context).to have_attributes(
trace_id: be == new_context.trace_id,
span_id: be == new_context.parent_id
)
end
end
end
end
Loading