getsentry / getsentry/sentry-ruby

Span#deep_dup is a shallow dup

Abierto
#3,039 1 comentario 0 reacciones 1 asignado Reclamado por @sl0thentr0py Ver en GitHub
Ruby Traces
Lenguaje dominante
Ruby
Estrellas
987
Forks
541
Merge medio
17 h 40 min
PR fusionados (30 d)
19

Descripción

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

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.