-
Notifications
You must be signed in to change notification settings - Fork 440
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.
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
ddtrace/tracer: dereference pointers to supported types in span.SetTag #2796
Conversation
BenchmarksBenchmark execution time: 2024-08-05 09:34:08 Comparing candidate commit 8c808b6 in PR branch Some scenarios are present only in baseline or only in candidate runs. If you didn't create or remove some scenarios in your branch, this maybe a sign of crashed benchmarks 💥💥💥 Scenarios present only in candidate:
Found 9 performance improvements and 0 performance regressions! Performance is the same for 46 metrics, 1 unstable metrics. scenario:BenchmarkSetTagMetric-24
scenario:BenchmarkSetTagString-24
scenario:BenchmarkSetTagStringer-24
|
The reported benchmark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing for dd-trace-go-guild (.gitlab-ci.yml)
Co-authored-by: Eliott Bouhana <47679741+eliottness@users.noreply.github.com>
What does this PR do?
Adds support for pointer values on setting tags, dereferencing them iff it's a pointer to a supported value in
span.SetTag
.Dereferencing more than one level of indirection is out of scope, as there is no way to do it without
reflect
and/or in an efficient way.It also adds some of the existing benchmarks for
SetTag
to make sure the modification doesn't introduce any unnecessary allocation or overhead. They were also fixed as they had an unrelated allocation captured by the benchmark tooling.Motivation
Protobuf generated code for nullable fields uses pointers. It's easy to forget to dereference the variable to pass the value to
SetTag
, leading to tags containing strings representing memory addresses instead of the desired value the pointer points to.Reviewer's Checklist
Unsure? Have a question? Request a review!