diff --git a/ddtrace/contrib/elasticsearch/transport.py b/ddtrace/contrib/elasticsearch/transport.py index 2fde725b0af..39716f1d422 100644 --- a/ddtrace/contrib/elasticsearch/transport.py +++ b/ddtrace/contrib/elasticsearch/transport.py @@ -49,8 +49,7 @@ def perform_request(self, method, url, params=None, body=None): _, data = result took = data.get("took") if took: - # TODO: move that to a metric instead - s.set_tag(metadata.TOOK, took) + s.set_metric(metadata.TOOK, int(took)) return result diff --git a/ddtrace/contrib/psycopg/connection.py b/ddtrace/contrib/psycopg/connection.py index a6168baff8f..f69e133d302 100644 --- a/ddtrace/contrib/psycopg/connection.py +++ b/ddtrace/contrib/psycopg/connection.py @@ -64,7 +64,7 @@ def execute(self, query, vars=None): try: return super(TracedCursor, self).execute(query, vars) finally: - s.set_tag("db.rowcount", self.rowcount) + s.set_metric("db.rowcount", self.rowcount) def callproc(self, procname, vars=None): """ just wrap the execution in a span """ diff --git a/ddtrace/contrib/redis/tracers.py b/ddtrace/contrib/redis/tracers.py index ad8720864db..fb3aaf554d2 100644 --- a/ddtrace/contrib/redis/tracers.py +++ b/ddtrace/contrib/redis/tracers.py @@ -63,9 +63,8 @@ def execute(self, *args, **kwargs): s.set_tags(_extract_conn_tags(self.connection_pool.connection_kwargs)) s.set_tags(self._datadog_meta) - # FIXME[leo]: convert to metric? - s.set_tag(redisx.PIPELINE_LEN, len(self.command_stack)) - s.set_tag(redisx.PIPELINE_AGE, time.time()-self._datadog_pipeline_creation) + s.set_metric(redisx.PIPELINE_LEN, len(self.command_stack)) + s.set_metric(redisx.PIPELINE_AGE, time.time()-self._datadog_pipeline_creation) return super(TracedPipeline, self).execute(self, *args, **kwargs) @@ -84,8 +83,7 @@ def immediate_execute_command(self, *args, **kwargs): s.set_tags(_extract_conn_tags(self.connection_pool.connection_kwargs)) s.set_tags(self._datadog_meta) - # FIXME[leo]: convert to metric? - s.set_tag(redisx.ARGS_LEN, len(args)) + s.set_metric(redisx.ARGS_LEN, len(args)) s.set_tag(redisx.IMMEDIATE_PIPELINE, True) @@ -113,8 +111,7 @@ def execute_command(self, *args, **options): s.set_tags(_extract_conn_tags(self.connection_pool.connection_kwargs)) s.set_tags(self._datadog_meta) - # FIXME[leo]: convert to metric? - s.set_tag(redisx.ARGS_LEN, len(args)) + s.set_metric(redisx.ARGS_LEN, len(args)) return super(TracedRedis, self).execute_command(*args, **options) diff --git a/ddtrace/span.py b/ddtrace/span.py index 97ab6c720e7..a015e50daac 100644 --- a/ddtrace/span.py +++ b/ddtrace/span.py @@ -1,4 +1,5 @@ import logging +import numbers import random import sys import time @@ -89,7 +90,7 @@ def set_tag(self, key, value): try: self.meta[key] = stringify(value) except Exception: - log.warning("error setting tag. ignoring", exc_info=True) + log.warning("error setting tag %s, ignoring it", key, exc_info=True) def get_tag(self, key): """ Return the given tag or None if it doesn't exist. @@ -110,6 +111,18 @@ def set_meta(self, k, v): def set_metas(self, kvs): self.set_tags(kvs) + def set_metric(self, key, value): + try: + # If the value isn't a typed as a number (ex: a string), try to cast it + if not isinstance(value, numbers.Number): + value = float(value) + self.metrics[key] = value + except Exception: + log.warning("error setting metric %s, ignoring it", key, exc_info=True) + + def get_metric(self, key): + return self.metrics.get(key) + def to_dict(self): d = { 'trace_id' : self.trace_id, @@ -131,6 +144,9 @@ def to_dict(self): if self.meta: d['meta'] = self.meta + if self.metrics: + d['metrics'] = self.metrics + if self.span_type: d['type'] = self.span_type diff --git a/tests/contrib/elasticsearch/test.py b/tests/contrib/elasticsearch/test.py index ebe37ee1df9..0a5ae52b5e5 100644 --- a/tests/contrib/elasticsearch/test.py +++ b/tests/contrib/elasticsearch/test.py @@ -88,5 +88,4 @@ def test_elasticsearch(self): eq_(span.get_tag(metadata.BODY).replace(" ", ""), '{"query":{"match_all":{}}}') eq_(set(span.get_tag(metadata.PARAMS).split('&')), {'sort=name%3Adesc', 'size=100'}) - self.assertTrue(int(span.get_tag(metadata.TOOK)) > 0) - + self.assertTrue(span.get_metric(metadata.TOOK) > 0) diff --git a/tests/contrib/redis/test.py b/tests/contrib/redis/test.py index 19c0175166f..45e3d05bd42 100644 --- a/tests/contrib/redis/test.py +++ b/tests/contrib/redis/test.py @@ -45,12 +45,18 @@ def test_basic_class(self): eq_(span.name, 'redis.command') eq_(span.span_type, 'redis') eq_(span.error, 0) - eq_(span.meta, {'out.host': u'localhost', 'redis.raw_command': u'GET cheese', 'out.port': u'6379', 'redis.args_length': u'2', 'out.redis_db': u'0'}) + eq_(span.meta, { + 'out.host': u'localhost', + 'redis.raw_command': u'GET cheese', + 'out.port': u'6379', + 'out.redis_db': u'0', + }) + eq_(span.get_metric('redis.args_length'), 2) eq_(span.resource, 'GET cheese') services = writer.pop_services() expected = { - self.SERVICE: {"app":"redis", "app_type":"db"} + self.SERVICE: {"app": "redis", "app_type": "db"} } eq_(services, expected) @@ -88,15 +94,15 @@ def test_basic_class_pipeline(self): span = spans[0] eq_(span.service, self.SERVICE) eq_(span.name, 'redis.pipeline') + eq_(span.resource, u'SET blah 32\nRPUSH foo éé\nHGETALL xxx') eq_(span.span_type, 'redis') eq_(span.error, 0) eq_(span.get_tag('out.redis_db'), '0') eq_(span.get_tag('out.host'), 'localhost') - ok_(float(span.get_tag('redis.pipeline_age')) > 0) - eq_(span.get_tag('redis.pipeline_length'), '3') eq_(span.get_tag('out.port'), '6379') - eq_(span.resource, u'SET blah 32\nRPUSH foo éé\nHGETALL xxx') eq_(span.get_tag('redis.raw_command'), u'SET blah 32\nRPUSH foo éé\nHGETALL xxx') + ok_(span.get_metric('redis.pipeline_age') > 0) + eq_(span.get_metric('redis.pipeline_length'), 3) def test_custom_class(self): class MyCustomRedis(redis.Redis): diff --git a/tests/test_span.py b/tests/test_span.py index 9322f3f177b..01aada71ed4 100644 --- a/tests/test_span.py +++ b/tests/test_span.py @@ -31,6 +31,32 @@ def test_tags(): } eq_(d["meta"], expected) +def test_set_valid_metrics(): + s = Span(tracer=None, name="foo") + s.set_metric("a", 0) + s.set_metric("b", -12) + s.set_metric("c", 12.134) + s.set_metric("d", 1231543543265475686787869123) + s.set_metric("e", "12.34") + d = s.to_dict() + expected = { + "a": 0, + "b": -12, + "c": 12.134, + "d": 1231543543265475686787869123, + "e": 12.34, + } + eq_(d["metrics"], expected) + + +def test_set_invalid_metric(): + s = Span(tracer=None, name="foo") + + # Set an invalid metric: shouldn't crash nor set any value + s.set_metric("a", "forty-twelve") + + eq_(s.get_metric("a"), None) + def test_tags_not_string(): # ensure we can cast as strings class Foo(object):