Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use array for transaction names and sources on scope #2324

Merged
merged 1 commit into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
- Decrease the default number of background worker threads by half ([#2305](https://github.com/getsentry/sentry-ruby/pull/2305))
- Fixes [#2297](https://github.com/getsentry/sentry-ruby/issues/2297)
- Don't mutate `enabled_environments` when using `Sentry::TestHelper` ([#2317](https://github.com/getsentry/sentry-ruby/pull/2317))
- Don't use array for transaction names and sources on scope ([#2324](https://github.com/getsentry/sentry-ruby/pull/2324))
- Fixes [#2257](https://github.com/getsentry/sentry-ruby/issues/2257)

### Internal

Expand Down
1 change: 1 addition & 0 deletions sentry-ruby/bin/console
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env ruby

require "bundler/setup"
require "debug"
require "sentry-ruby"

# You can add fixtures and/or initialization code here to make experimenting
Expand Down
34 changes: 10 additions & 24 deletions sentry-ruby/lib/sentry/scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ class Scope
include ArgumentCheckingHelper

ATTRIBUTES = [
:transaction_names,
:transaction_sources,
:transaction_name,
:transaction_source,
:contexts,
:extra,
:tags,
Expand Down Expand Up @@ -96,8 +96,8 @@ def dup
copy.extra = extra.deep_dup
copy.tags = tags.deep_dup
copy.user = user.deep_dup
copy.transaction_names = transaction_names.dup
copy.transaction_sources = transaction_sources.dup
copy.transaction_name = transaction_name.dup
copy.transaction_source = transaction_source.dup
copy.fingerprint = fingerprint.deep_dup
copy.span = span.deep_dup
copy.session = session.deep_dup
Expand All @@ -114,8 +114,8 @@ def update_from_scope(scope)
self.extra = scope.extra
self.tags = scope.tags
self.user = scope.user
self.transaction_names = scope.transaction_names
self.transaction_sources = scope.transaction_sources
self.transaction_name = scope.transaction_name
self.transaction_source = scope.transaction_source
self.fingerprint = scope.fingerprint
self.span = scope.span
self.propagation_context = scope.propagation_context
Expand Down Expand Up @@ -231,8 +231,8 @@ def set_level(level)
# @param transaction_name [String]
# @return [void]
def set_transaction_name(transaction_name, source: :custom)
@transaction_names << transaction_name
@transaction_sources << source
@transaction_name = transaction_name
@transaction_source = source
end

# Sets the currently active session on the scope.
Expand All @@ -242,20 +242,6 @@ def set_session(session)
@session = session
end

# Returns current transaction name.
# The "transaction" here does not refer to `Transaction` objects.
# @return [String, nil]
def transaction_name
@transaction_names.last
end

# Returns current transaction source.
# The "transaction" here does not refer to `Transaction` objects.
# @return [String, nil]
def transaction_source
@transaction_sources.last
end

# These are high cardinality and thus bad.
# @return [Boolean]
def transaction_source_low_quality?
Expand Down Expand Up @@ -311,8 +297,8 @@ def set_default_value
@user = {}
@level = :error
@fingerprint = []
@transaction_names = []
@transaction_sources = []
@transaction_name = nil
@transaction_source = nil
@event_processors = []
@rack_env = {}
@span = nil
Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
event = last_sentry_event
expect(event.transaction).to eq("/test")
expect(event.to_hash.dig(:request, :url)).to eq("http://example.org/test")
expect(Sentry.get_current_scope.transaction_names).to be_empty
expect(Sentry.get_current_scope.transaction_name).to be_nil
expect(Sentry.get_current_scope.rack_env).to eq({})
end

Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/spec/sentry/scope/setters_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@
end

describe "#set_transaction_name" do
it "pushes the transaction_name to transaction_names stack" do
it "sets the transaction name" do
subject.set_transaction_name("WelcomeController#home")

expect(subject.transaction_name).to eq("WelcomeController#home")
Expand Down
15 changes: 7 additions & 8 deletions sentry-ruby/spec/sentry/scope_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
expect(subject.tags).to eq({})
expect(subject.user).to eq({})
expect(subject.fingerprint).to eq([])
expect(subject.transaction_names).to eq([])
expect(subject.transaction_sources).to eq([])
expect(subject.transaction_name).to eq(nil)
expect(subject.transaction_source).to eq(nil)
expect(subject.propagation_context).to be_a(Sentry::PropagationContext)
end

Expand All @@ -42,8 +42,7 @@
copy.extra.merge!(foo: "bar")
copy.tags.merge!(foo: "bar")
copy.user.merge!(foo: "bar")
copy.transaction_names << "foo"
copy.transaction_sources << :url
copy.set_transaction_name("foo", source: :url)
copy.fingerprint << "bar"

expect(subject.breadcrumbs.to_hash).to eq({ values: [] })
Expand All @@ -53,8 +52,8 @@
expect(subject.tags).to eq({})
expect(subject.user).to eq({})
expect(subject.fingerprint).to eq([])
expect(subject.transaction_names).to eq([])
expect(subject.transaction_sources).to eq([])
expect(subject.transaction_name).to eq(nil)
expect(subject.transaction_source).to eq(nil)
expect(subject.span).to eq(nil)
end

Expand Down Expand Up @@ -143,8 +142,8 @@
expect(subject.tags).to eq({})
expect(subject.user).to eq({})
expect(subject.fingerprint).to eq([])
expect(subject.transaction_names).to eq([])
expect(subject.transaction_sources).to eq([])
expect(subject.transaction_name).to eq(nil)
expect(subject.transaction_source).to eq(nil)
expect(subject.span).to eq(nil)
end
end
Expand Down
Loading