Track the number of allocations made in each span#597
Conversation
e86f0e8 to
da8c8c0
Compare
|
The tests are failing because the Span class has too many lines now – can I ignore that error? |
|
@dasch You can add a |
There was a problem hiding this comment.
I'm digging this feature! But just a couple of general questions:
- How accurate/reliable is
GC.statfor this? From what I remember playing around with this, it wasn't very deterministic. (Because other threads in the same process can allocate memory in parallel which changes this stat?) However, I can see this is exactly what the Rails guys did, so I presume they did their homework on this. I suppose as long as its in the ballpark and its generally useful even if not perfectly accurate, I'd be willing to give it the benefit of the doubt. - Does this work for Ruby 1.9.3+? (JRuby excepted?) If not we might need to add in some compatibility or short circuit for it.
There was a problem hiding this comment.
- It seems to be good enough to use in Rails, and it's it doesn't have to be 100% precise to be useful in practice, especially if you aggregate.
- I've added a compatibility check.
cdc1ad2 to
7dfcc99
Compare
There was a problem hiding this comment.
@dasch This is looking good to me overall. I gave this a test through a sample app, seems to be working fine.
In general I think we want to be careful with what's being sent to the agent, as it can reject some unexpected data, or increase the size of payloads. However the impact here is pretty small, and the traces were coming through okay, so this should be good to merge.
It might be a good idea to consider this a "beta" feature while we experiment with it, as I think we could reasonably expect our support for memory statistics and API for such features grow in the near future, and this might need to evolve and be incorporated into that. Nonetheless this is a great first step in that direction.
Thanks so much for the contribution, looking forward to trying this out more! 👍
delner
added
core
|
@dasch Can you rebase this against |
4ee1e94 to
b8f7d9d
Compare
|
@delner I've squashed and rebased on 0.17-dev. Not sure how to make this a beta feature, I assume you can figure that out once this has been merged. |
|
@dasch Yeah, we're just going to mark it as such in release notes. Thank you! |
**What does this PR do?**:
This PR adds a new profiler public API:
`Datadog::Profiling.allocation_count`.
The public documentation for this API is as follows:
> Returns an ever-increasing counter of the number of allocations
> observed by the profiler in this thread.
>
> Note 1: This counter may not start from zero on new threads. It
> should only be used to measure how many
> allocations have happened between two calls to this API:
> ```ruby
> allocations_before = Datadog::Profiling.allocation_count
> do_some_work()
> allocations_after = Datadog::Profiling.allocation_count
> puts "Allocations during do_some_work: #{allocations_after - allocations_before}"
> ```
> (This is similar to some OS-based time representations.)
>
> Note 2: All fibers in the same thread will share the same counter
> values.
>
> Only available when the profiler is running, the new CPU Profiling
> 2.0 profiler is in use, and allocation-related
> features are not disabled via configuration.
> For instructions on enabling CPU Profiling 2.0 see the ddtrace
> release notes.
As long as CPU Profiling 2.0 is in use, this API is enabled by
default. To disable it, this PR adds a new setting:
```ruby
Datadog.configure do |c|
c.profiling.advanced.allocation_counting_enabled = # ...
end
```
**Motivation**:
This feature has long been something we want to provide with ddtrace,
see issues #2164 and #468, as well as PRs #1891, #1805, #597
As part of the ongoing work of enabling allocation profiling,
counting the number of allocations comes at a very cheap cost since
the profiler needs to have a `RUBY_INTERNAL_EVENT_NEWOBJ`
tracepoint anyway -- it's just a matter of also incrementing a
counter inside it.
**Additional Notes**:
Note that this does not yet change any user-visible feature for
ddtrace. I'm counting on @marcotc to pick up the task of using this
API to make some tracing magic :)
**How to test the change?**:
This change includes code coverage.
---
Fixes #2164
2 participants
Inspired by rails/rails@42fec4b, this change starts capturing the number of allocations made in each span.
I'm not sure how you'd like to integrate this data – do you just want to use a tag, or would you prefer a first-class attribute on Span?