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.

Sign up for GitHub

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

Automatically add the pid of the current process to root spans #147

Closed
wants to merge 3 commits into from
Closed

Automatically add the pid of the current process to root spans #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
35a7aed
Add pid to root spans
bmermet Jul 5, 2017
6d62fb5
Rewrite tests for compatibility with older ruby version
bmermet Jul 6, 2017
2c621ef
Moved system.pid to an ext variable
bmermet Jul 6, 2017
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
7 changes: 7 additions & 0 deletions lib/ddtrace/ext/system.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module Datadog
module Ext
module SYSTEM
PID = 'system.pid'.freeze
end
end
end
2 changes: 2 additions & 0 deletions lib/ddtrace/tracer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
require 'ddtrace/logger'
require 'ddtrace/writer'
require 'ddtrace/sampler'
require 'ddtrace/ext/system'

# \Datadog global namespace that includes all tracing functionality for Tracer and Span classes.
module Datadog
Expand Down Expand Up @@ -161,6 +162,7 @@ def start_span(name, options = {})
# root span
span = Span.new(self, name, opts)
@sampler.sample(span)
span.set_tag(Datadog::Ext::SYSTEM::PID, Process.pid)
else
# child span
opts[:service] ||= parent.service
Expand Down
23 changes: 21 additions & 2 deletions test/tracer_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,10 @@ def test_trace_all_args
assert_equal('extra-resource', span.resource)
assert_equal('my-type', span.span_type)
assert_equal(yesterday, span.start_time)
assert_equal({ 'env' => 'test', 'temp' => 'cool', 'tag1' => 'value1', 'tag2' => 'value2' }, span.meta)
assert_equal('test', span.get_tag('env'))
assert_equal('cool', span.get_tag('temp'))
assert_equal('value1', span.get_tag('tag1'))
assert_equal('value2', span.get_tag('tag2'))
Copy link
Contributor

Choose a reason for hiding this comment

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

mmm, here, to be truely conform to the previous test, we should also assert_equal(4, span.meta.length) to make sure no extra tag creep in. But this is a nitpick.

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 left it out on purpose. I have mixed feelings about it. On one hand it's nice to check more tags haven't been added, on the other hand it means that adding automatic tags like the pid here means having to update unrelated tests. If you think that the pros outweigh the cons, I'll add that check.

end

def test_start_span_all_args
Expand All @@ -193,7 +196,10 @@ def test_start_span_all_args
assert_equal('extra-resource', span.resource)
assert_equal('my-type', span.span_type)
assert_equal(yesterday, span.start_time)
assert_equal({ 'env' => 'test', 'temp' => 'cool', 'tag1' => 'value1', 'tag2' => 'value2' }, span.meta)
assert_equal('test', span.get_tag('env'))
assert_equal('cool', span.get_tag('temp'))
assert_equal('value1', span.get_tag('tag1'))
assert_equal('value2', span.get_tag('tag2'))
end

def test_start_span_child_of_span
Expand Down Expand Up @@ -273,4 +279,17 @@ def test_start_span_child_of_context
assert_equal(a.trace_id, c.trace_id, 'a and c belong to the same trace')
assert_equal(a.span_id, c.parent_id, 'a is the parent of c')
end

def test_root_span_has_pid_metadata
tracer = get_test_tracer
root = tracer.trace('something')
assert_equal(Process.pid.to_s, root.get_tag(Datadog::Ext::SYSTEM::PID))
end

def test_child_span_has_no_pid_metadata
tracer = get_test_tracer
tracer.trace('something')
child = tracer.trace('something_else')
assert_nil(child.get_tag(Datadog::Ext::SYSTEM::PID))
end
end