getsentry / getsentry/sentry-ruby

Span#deep_dup is a shallow dup

Offen
#3,039 1 Kommentar 0 Reaktionen 1 zugewiesene Person Beansprucht von @sl0thentr0py Auf GitHub ansehen
Ruby Traces
Vorherrschende Sprache
Ruby
Sterne
987
Forks
541
Ø Merge
17 Std. 40 Min.
Gemergte PRs (30 T.)
19

Beschreibung

`Span#deep_dup` (span.rb:245) is `def deep_dup; dup; end`. The copy shares `@data`, `@tags`, and `@span_recorder` with the original, so writing to the copy mutates the original.

`Scope#dup` calls it on every `push_scope` and every `capture_event`, so a "copied" scope holding a plain child span shares the parent's recorder.

Making it a real deep copy changes allocation behaviour on a hot path. Happy to open a PR to fix this once maintainers weigh in

Failing spec

Drop in `sentry-ruby/spec/` and run `bundle exec rspec spec/span_deep_dup_spec.rb`. Both examples fail on master.

```ruby
# Run from sentry-ruby/: bundle exec rspec path/to/span_deep_dup_spec.rb
require "spec_helper"

RSpec.describe "Span#deep_dup" do
before { perform_basic_setup { |config| config.traces_sample_rate = 1.0 } }

let(:span) do
transaction = Sentry.start_transaction(name: "t", op: "op")
transaction.start_child(op: "child")
end

it "does not share mutable state with the original" do
span.set_data("original", true)
copy = span.deep_dup

copy.set_data("written_via_copy", true)

expect(span.data).not_to have_key("written_via_copy")
end

it "gives the copy its own data, tags and span recorder" do
copy = span.deep_dup

aggregate_failures do
expect(copy.data).not_to be(span.data)
expect(copy.tags).not_to be(span.tags)
expect(copy.span_recorder).not_to be(span.span_recorder)
end
end
end
```

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.