getsentry / getsentry/sentry-ruby

Span#deep_dup is a shallow dup

オープン
#3,039 コメント 1 件 リアクション 0 件 担当者 1 名 @sl0thentr0py が担当を希望しています GitHub で見る
Ruby Traces
主要言語
Ruby
スター
987
フォーク
541
平均マージ
17時間 40分
マージ済み PR(30日)
19

説明

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

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。