perf: cache trace_id_64bits when encoding (#14034)

Avoiding accessing the `Span.trace_id_64bits` computed per-span, when
the value is the same for every encoded trace.

This change accesses the trace id once and passes along to every
`pack_span` call.

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [ ] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

---------

Co-authored-by: kyle <[email protected]>
Co-authored-by: Christophe Papazian <[email protected]>
Co-authored-by: Vítor De Araújo <[email protected]>
Co-authored-by: Emmett Butler <[email protected]>
Co-authored-by: jsimpher <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Taegyun Kim <[email protected]>

Author: 8 people

ddtrace/internal/_encoding.pyx

@@ -498,6 +498,7 @@ cdef class MsgpackEncoderBase(BufferedEncoder):
         cdef int ret
         cdef Py_ssize_t L
         cdef void * dd_origin = NULL
+        cdef unsigned long long trace_id_64bits = 0
 
         L = len(trace)
         if L > ITEM_LIMIT:
@@ -507,12 +508,26 @@ cdef class MsgpackEncoderBase(BufferedEncoder):
         if ret != 0:
             raise RuntimeError("Couldn't pack trace")
 
-        if L > 0 and trace[0].context is not None and trace[0].context.dd_origin is not None:
+        # TODO: Can we skip packing an empty array?
+        if L == 0:
+            return 0
+
+        if trace[0].context is not None and trace[0].context.dd_origin is not None:
             dd_origin = self.get_dd_origin_ref(trace[0].context.dd_origin)
 
+        # PERF: _trace_id_64bits is a computed property, cache/convert once for all spans
+        try:
+            trace_id_64bits = trace[0]._trace_id_64bits
+
+        # We can get TypeError if the trace_id is not an int
+        #   e.g. "unsupported operand type(s) for &: 'int' and 'str'"
+        except Exception as e:
+            raise RuntimeError(
+                "failed to pack span: {!r}. Exception: {}".format(trace[0], e)
+            )
         for span in trace:
             try:
-                ret = self.pack_span(span, dd_origin)
+                ret = self.pack_span(span, trace_id_64bits, dd_origin)
             except Exception as e:
                 raise RuntimeError("failed to pack span: {!r}. Exception: {}".format(span, e))
 
@@ -562,7 +577,7 @@ cdef class MsgpackEncoderBase(BufferedEncoder):
     cpdef flush(self):
         raise NotImplementedError()
 
-    cdef int pack_span(self, object span, void *dd_origin) except? -1:
+    cdef int pack_span(self, object span, unsigned long long trace_id_64bits, void *dd_origin) except? -1:
         raise NotImplementedError()
 
 
@@ -744,7 +759,7 @@ cdef class MsgpackEncoderV04(MsgpackEncoderBase):
 
         raise TypeError("Unhandled metrics type: %r" % type(metrics))
 
-    cdef int pack_span(self, object span, void *dd_origin) except? -1:
+    cdef int pack_span(self, object span, unsigned long long trace_id_64bits, void *dd_origin) except? -1:
         cdef int ret
         cdef Py_ssize_t L
         cdef int has_span_type
@@ -775,7 +790,7 @@ cdef class MsgpackEncoderV04(MsgpackEncoderBase):
             ret = pack_bytes(&self.pk, <char *> b"trace_id", 8)
             if ret != 0:
                 return ret
-            ret = pack_number(&self.pk, span._trace_id_64bits)
+            ret = msgpack_pack_unsigned_long_long(&self.pk, trace_id_64bits)
             if ret != 0:
                 return ret
 
@@ -1020,7 +1035,7 @@ cdef class MsgpackEncoderV05(MsgpackEncoderBase):
     cdef void * get_dd_origin_ref(self, str dd_origin):
         return <void *> PyLong_AsLong(self._st._index(dd_origin))
 
-    cdef int pack_span(self, object span, void *dd_origin) except? -1:
+    cdef int pack_span(self, object span, unsigned long long trace_id_64bits, void *dd_origin) except? -1:
         cdef int ret
 
         ret = msgpack_pack_array(&self.pk, 12)
@@ -1037,8 +1052,7 @@ cdef class MsgpackEncoderV05(MsgpackEncoderBase):
         if ret != 0:
             return ret
 
-        _ = span._trace_id_64bits
-        ret = msgpack_pack_uint64(&self.pk, _ if _ is not None else 0)
+        ret = msgpack_pack_unsigned_long_long(&self.pk, trace_id_64bits)
         if ret != 0:
             return ret
 

tests/tracer/test_encoders.py

@@ -70,6 +70,8 @@ def gen_trace(nspans=1000, ntags=50, key_size=15, value_size=20, nmetrics=10):
             resource="/fsdlajfdlaj/afdasd%s" % i,
             service="myservice",
             parent_id=parent_id,
+            # All spans in a trace must share a trace_id
+            trace_id=root.trace_id if root else None,
         ) as span:
             span._parent = root
             span.set_tags({rands(key_size): rands(value_size) for _ in range(0, ntags)})