Skip to content

Remove SpanOperation.allocations#1891

Merged
marcotc merged 1 commit into
1.0from
remove-span-allocations
Feb 9, 2022
Merged

Remove SpanOperation.allocations#1891
marcotc merged 1 commit into
1.0from
remove-span-allocations

Conversation

@marcotc

@marcotc marcotc commented Feb 8, 2022

Copy link
Copy Markdown
Member

The allocations metric on Span has been around for a long time, sitting unused, but only recently exposed as a metric.

Because we only collect global statistics, the number of allocations reported on each span can be grossly exaggerated. Besides, it has the highest cost of any operation in the SpanOperation class, affecting the critical path performance.

Breaking change

  • Removed: span allocations metric.

@marcotc
marcotc requested a review from a team February 8, 2022 19:13
@marcotc
marcotc marked this pull request as draft February 8, 2022 19:14
@marcotc
marcotc force-pushed the remove-event-mutex branch from 2a91615 to 2142840 Compare February 9, 2022 00:36
@marcotc
marcotc changed the base branch from remove-event-mutex to 1.0 February 9, 2022 00:38
@marcotc
marcotc force-pushed the remove-span-allocations branch from 4cfb5ea to af4a46d Compare February 9, 2022 00:39
@marcotc marcotc self-assigned this Feb 9, 2022
@marcotc marcotc added breaking-change Involves a breaking change core Involves Datadog core libraries performance Involves performance (e.g. CPU, memory, etc) labels Feb 9, 2022
@marcotc marcotc added this to the 1.0.0 milestone Feb 9, 2022
@marcotc
marcotc marked this pull request as ready for review February 9, 2022 00:40
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #1891 (af4a46d) into 1.0 (a46f2d9) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              1.0    #1891      +/-   ##
==========================================
- Coverage   98.23%   98.22%   -0.02%     
==========================================
  Files         966      966              
  Lines       47931    47897      -34     
==========================================
- Hits        47084    47045      -39     
- Misses        847      852       +5     
Impacted Files Coverage Δ
lib/datadog/tracing/span_operation.rb 81.21% <ø> (-3.56%) ⬇️
spec/datadog/tracing/span_operation_spec.rb 100.00% <ø> (ø)
lib/datadog/tracing/span.rb 96.20% <0.00%> (-1.27%) ⬇️

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 a46f2d9...af4a46d. Read the comment docs.

@ivoanjo ivoanjo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Slightly-sadface-emoji lol.

I had recently added #1805 after discussing it with a customer, you make a good point that this value can be misleading on multi-threaded Ruby apps.

Anyway, we hope to provide a better alternative via profiling instead, so let's take the speed improvement :)

@marcotc
marcotc merged commit d4ea79e into 1.0 Feb 9, 2022
@marcotc
marcotc deleted the remove-span-allocations branch February 9, 2022 18:01
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

breaking-change Involves a breaking change core Involves Datadog core libraries performance Involves performance (e.g. CPU, memory, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants