ddtrace/tracer: encode span IDs in execution traces efficiently #2268
Conversation
We were previously encoding span IDs into execution traces as base-10 strings. This is wasteful, in terms of CPU time to encode the ID and more importantly in terms of how much of the limited execution trace data the profiler collects will be taken up by span IDs. They can be encoded directly as 8-byte unsigned integers.
BenchmarksBenchmark execution time: 2023-10-23 13:07:04 Comparing candidate commit 33dc21c in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 41 metrics, 0 unstable metrics. |
Check that we create the appropriate span annotations in execution traces. This test is dependent on parsing an execution trace, which can change format across releases. We don't have a standard library parser. We rely on an external implementation. To prevent this test from breaking in the window between when a new Go version comes out and when the parser supports any format changes, guard it with the supported Go version. We support several Go versions, so we should have some coverage all the time.
| // TODO: can we make string(b[:]) not allocate? e.g. with unsafe | ||
| // shenanigans? rt.Log won't retain the message string, though perhaps | ||
| // we can't assume that will always be the case. | ||
| rt.Log(ctx, "datadog.uint64_span_id", string(b[:])) |
There was a problem hiding this comment.
From Go 1.20 we can use unsafe.String but looking at the repository and from what I heard, I think that we don't want unsafe calls in the tracer.
There was a problem hiding this comment.
@darccio 👍
Let's not worry about it as part of this PR. The previous code already caused an allocation as well as far as I can tell. We can revisit this for flight recording mode.
| // TODO: can we make string(b[:]) not allocate? e.g. with unsafe | ||
| // shenanigans? rt.Log won't retain the message string, though perhaps | ||
| // we can't assume that will always be the case. | ||
| rt.Log(ctx, "datadog.uint64_span_id", string(b[:])) |
There was a problem hiding this comment.
@darccio 👍
Let's not worry about it as part of this PR. The previous code already caused an allocation as well as far as I can tell. We can revisit this for flight recording mode.
What does this PR do?
Encode span IDs directly as 8-byte unsigned integers in the execution trace log
message.
Add a unit test that we create the appropriate span annotations in execution
traces. This test is dependent on parsing an execution trace, which can
change format across releases. We don't have a standard library parser.
We rely on an external implementation. To prevent this test from
breaking in the window between when a new Go version comes out and when
the parser supports any format changes, guard it with the supported Go
version. We support several Go versions, so we should have some coverage
all the time.
cc @felixge
Motivation
We were previously encoding span IDs into execution traces as base-10
strings. This is wasteful, in terms of CPU time to encode the ID and
more importantly in terms of how much of the limited execution trace
data the profiler collects will be taken up by span IDs. They can be
encoded directly as 8-byte unsigned integers.
Reviewer's Checklist
For Datadog employees:
@DataDog/security-design-and-guidance.Unsure? Have a question? Request a review!