feat: store tracer configuration in an in-memory file#184
Conversation
Resolves [APMAPI-1066]
BenchmarksBenchmark execution time: 2025-02-11 13:24:28 Comparing candidate commit 6f71825 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #184 +/- ##
==========================================
- Coverage 93.80% 93.62% -0.18%
==========================================
Files 73 73
Lines 4195 4252 +57
==========================================
+ Hits 3935 3981 +46
- Misses 260 271 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pablomartinezbernardo
left a comment
pablomartinezbernardo
left a comment
There was a problem hiding this comment.
LGTM, because although there's a couple of questions, being aware of those concerns is enough
| } | ||
|
|
||
| Expected<InMemoryFile> InMemoryFile::make(StringView name) { | ||
| int fd = memfd_create(name.data(), MFD_CLOEXEC | MFD_ALLOW_SEALING); |
There was a problem hiding this comment.
Question: The RFC suggest to use libdatadog for this. Is this temporary or will the cpp tracer now follow that advice?
There was a problem hiding this comment.
It suggest, doesn't enforce. Since dd-trace-cpp has no dependency on libdatadog, it will not be that efficient to use it.
| return config.dump(); | ||
| } | ||
|
|
||
| void Tracer::store_config() { |
There was a problem hiding this comment.
Can this lead to incorrect usage? If store_config is called twice, will that result in multiple descriptors for this process? If so, what are the implications of that?
There was a problem hiding this comment.
Every call to this method will result in N file descriptors. Since Tracer::store_config() is a private method and we control how it’s called, I believe we can ensure it’s used correctly and avoid any unintended issues.
3 participants
Description
Resolves APMAPI-1066