getsentry / getsentry/sentry-ruby

Span#deep_dup is a shallow dup

Aperta
#3,039 1 commento 0 reazioni 1 assegnatario Rivendicata da @sl0thentr0py Vedi su GitHub
Ruby Traces
Lingua principale
Ruby
Stelle
987
Fork
541
Merge medio
17h 40m
PR unite (30g)
19

Descrizione

`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
```

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.