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

Jump to bottom

[core] add metrics testing #81

Merged
merged 5 commits into from
Mar 6, 2017
Merged

[core] add metrics testing #81

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d9e802e
[sampler] disable logger on tests
Mar 6, 2017
332a929
[core] set_metric enforces float values, recovering if the cast is no…
Mar 6, 2017
ef07ab5
[metrics] add integration tests for Span metrics
Mar 6, 2017
132a404
[core] rubocop cosmetics
Mar 6, 2017
132aa4f
[docs] set_metric is available in the Manual Instrumentation section
Mar 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
6 changes: 5 additions & 1 deletion lib/ddtrace/span.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def initialize(tracer, name, options = {})
def set_tag(key, value)
@meta[key] = value.to_s
rescue StandardError => e
Datadog::Tracer.log.error("Unable to set the tag #{key}, ignoring it. Caused by: #{e}")
Datadog::Tracer.log.debug("Unable to set the tag #{key}, ignoring it. Caused by: #{e}")
end

# Return the tag with the given key, nil if it doesn't exist.
Expand All @@ -68,7 +68,11 @@ def get_tag(key)
# Set the given key / value metric pair on the span. Keys must be string.
# Values must be floating point numbers.
def set_metric(key, value)
# enforce that the value is a floating point number
value = Float(value)
@metrics[key] = value
rescue StandardError => e
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that a cast will raise an ArgumentError.
A cleaner way would be to check if value.is_a?(Integer) || value.is_a?(Float).

Copy link
Contributor Author

@palazzem palazzem Mar 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

if nil is passed it will be a TypeError:

irb(main):001:0> Float(nil)
TypeError: can't convert nil into Float
	from (irb):1:in `Float'
	from (irb):1
	from .../versions/2.3.1/bin/irb:11:in `<main>'

The reason to use the Float(value) is that a string may be converted to float, as we're doing in the Python implementation. I think it was already discussed before about why we should at least try to make it work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok then.

Datadog::Tracer.log.debug("Unable to set the metric #{key}, ignoring it. Caused by: #{e}")
end

# Return the metric with the given key, nil if it doesn't exist.
Expand Down
37 changes: 37 additions & 0 deletions test/span_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,41 @@ def test_span_set_parent_nil
assert_equal(child.parent_id, 0)
assert_equal(child.service, 'defaultdb')
end

def test_get_valid_metric
span = Datadog::Span.new(nil, 'test.span')
span.set_metric('a', 10)
assert_equal(10.0, span.get_metric('a'))
end

def test_set_valid_metrics
# metrics must be converted to float values
span = Datadog::Span.new(nil, 'test.span')
span.set_metric('a', 0)
span.set_metric('b', -12)
span.set_metric('c', 12.134)
span.set_metric('d', 1231543543265475686787869123)
span.set_metric('e', '12.34')
h = span.to_hash
expected = {
'a' => 0.0,
'b' => -12.0,
'c' => 12.134,
'd' => 1231543543265475686787869123.0,
'e' => 12.34,
}
assert_equal(expected, h[:metrics])
end

def test_invalid_metrics
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

# invalid values must be discarded
span = Datadog::Span.new(nil, 'test.span')
span.set_metric('a', nil)
span.set_metric('b', {})
span.set_metric('c', [])
span.set_metric('d', span)
span.set_metric('e', 'a_string')
h = span.to_hash
assert_equal({}, h[:metrics])
end
end