Skip to content
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

Send number of allocations in span as metric #1805

Merged
merged 1 commit into from
Dec 9, 2021

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Dec 6, 2021

I recently noticed that while we usually collect the number of memory allocations on a given span, we send it as an "allocations" field to the agent and this data never seems to make it to the backend.

Instead, this refactor reports it as a span metric. Does this make sense?

This is still missing tests and whatnot, but I wanted to get a feel if I'm doing this correctly before I did the full changes.

Also, as requested by @delner I'm targeting this to the 1.0 branch.

@ivoanjo ivoanjo requested a review from a team December 6, 2021 11:11
I recently noticed that while we usually collect the number of memory
allocations on a given span, we send it as an "allocations" field to
the agent and this data never seems to make it to the backend.

Instead, this refactor reports it as a span metric, which makes
it available to customers.
@ivoanjo ivoanjo force-pushed the ivoanjo/rfc-allocations-as-metric branch from 1a2ffac to 59e8449 Compare December 8, 2021 11:04
@codecov-commenter
Copy link

Codecov Report

Merging #1805 (59e8449) into 1.0 (422584e) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              1.0    #1805      +/-   ##
==========================================
+ Coverage   98.16%   98.18%   +0.02%     
==========================================
  Files         943      943              
  Lines       46167    46171       +4     
==========================================
+ Hits        45318    45332      +14     
+ Misses        849      839      -10     
Impacted Files Coverage Δ
lib/ddtrace/span.rb 97.43% <ø> (+1.18%) ⬆️
spec/ddtrace/span_spec.rb 100.00% <ø> (ø)
lib/ddtrace/span_operation.rb 83.78% <100.00%> (+4.00%) ⬆️
lib/ddtrace/transport/serializable_trace.rb 98.11% <100.00%> (-0.04%) ⬇️
spec/ddtrace/span_operation_spec.rb 100.00% <100.00%> (+0.37%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 422584e...59e8449. Read the comment docs.

@ivoanjo ivoanjo requested a review from marcotc December 8, 2021 11:32
@ivoanjo
Copy link
Member Author

ivoanjo commented Dec 8, 2021

@marcotc Care to give it another quick check? I've added/fixed the tests.

Copy link
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ivoanjo ivoanjo merged commit ad3d318 into 1.0 Dec 9, 2021
@ivoanjo ivoanjo deleted the ivoanjo/rfc-allocations-as-metric branch December 9, 2021 08:32
@ivoanjo ivoanjo changed the title RFC: Send number of allocations in span as metric Send number of allocations in span as metric Dec 9, 2021
@delner delner added this to the 1.0.0 milestone Jan 22, 2022
ivoanjo added a commit that referenced this pull request Feb 20, 2023
**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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants