Skip to content

Commit f82a2c3

Browse files
authored
Make hub a required argument for Transaction constructor (#1401)
* Make hub a required argument for Transaction construbtor See the proposal in #1393 * Add deprecation warning about passing hub in `Transaction#finish` * Update sentry-rails' specs * Update changelog
1 parent 36f9bed commit f82a2c3

File tree

12 files changed

+52
-41
lines changed

12 files changed

+52
-41
lines changed

sentry-rails/spec/sentry/rails/tracing/action_controller_subscriber_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
end
3838

3939
it "doesn't record spans" do
40-
transaction = Sentry::Transaction.new(sampled: false)
40+
transaction = Sentry::Transaction.new(sampled: false, hub: Sentry.get_current_hub)
4141
Sentry.get_current_scope.set_span(transaction)
4242

4343
get "/world"

sentry-rails/spec/sentry/rails/tracing/action_view_subscriber_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
end
3636

3737
it "doesn't record spans" do
38-
transaction = Sentry::Transaction.new(sampled: false)
38+
transaction = Sentry::Transaction.new(sampled: false, hub: Sentry.get_current_hub)
3939
Sentry.get_current_scope.set_span(transaction)
4040

4141
get "/view"

sentry-rails/spec/sentry/rails/tracing/active_record_subscriber_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
end
1515

1616
it "records database query events" do
17-
transaction = Sentry::Transaction.new(sampled: true)
17+
transaction = Sentry::Transaction.new(sampled: true, hub: Sentry.get_current_hub)
1818
Sentry.get_current_scope.set_span(transaction)
1919

2020
Post.all.to_a
@@ -40,7 +40,7 @@
4040
end
4141

4242
it "doesn't record spans" do
43-
transaction = Sentry::Transaction.new(sampled: false)
43+
transaction = Sentry::Transaction.new(sampled: false, hub: Sentry.get_current_hub)
4444
Sentry.get_current_scope.set_span(transaction)
4545

4646
Post.all.to_a

sentry-rails/spec/sentry/rails/tracing_spec.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@
143143
op: "pageload",
144144
status: "ok",
145145
sampled: true,
146-
name: "a/path"
146+
name: "a/path",
147+
hub: Sentry.get_current_hub
147148
)
148149
end
149150

sentry-ruby/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ The SDK now records a new `net.http` breadcrumb whenever the user makes a reques
2323
- Let Transaction constructor take an optional hub argument [#1384](https://github.com/getsentry/sentry-ruby/pull/1384)
2424
- Introduce LoggingHelper [#1385](https://github.com/getsentry/sentry-ruby/pull/1385)
2525
- Raise exception if a Transaction is initialized without a hub [#1391](https://github.com/getsentry/sentry-ruby/pull/1391)
26+
- Make hub a required argument for Transaction constructor [#1401](https://github.com/getsentry/sentry-ruby/pull/1401)
2627

2728
## 4.3.2
2829

sentry-ruby/lib/sentry/transaction.rb

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,13 @@ class Transaction < Span
1414

1515
attr_reader :name, :parent_sampled, :hub, :configuration, :logger
1616

17-
def initialize(name: nil, parent_sampled: nil, hub: Sentry.get_current_hub, **options)
17+
def initialize(name: nil, parent_sampled: nil, hub:, **options)
1818
super(**options)
1919

2020
@name = name
2121
@parent_sampled = parent_sampled
2222
@transaction = self
2323
@hub = hub
24-
25-
raise Sentry::Error.new("please initialize the SDK with Sentry.init before initializing a Transaction") unless @hub
26-
2724
@configuration = hub.configuration
2825
@logger = configuration.logger
2926
init_span_recorder
@@ -115,6 +112,15 @@ def set_initial_sample_decision(sampling_context:)
115112
end
116113

117114
def finish(hub: nil)
115+
if hub
116+
log_warn(
117+
<<~MSG
118+
Specifying a different hub in `Transaction#finish` will be deprecated in version 5.0.
119+
Please use `Hub#start_transaction` with the designated hub.
120+
MSG
121+
)
122+
end
123+
118124
hub ||= @hub
119125

120126
super() # Span#finish doesn't take arguments

sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,8 @@
162162
op: "pageload",
163163
status: "ok",
164164
sampled: true,
165-
name: "a/path"
165+
name: "a/path",
166+
hub: Sentry.get_current_hub
166167
)
167168
end
168169
let(:stack) do
@@ -396,7 +397,8 @@ def will_be_sampled_by_sdk
396397
op: "pageload",
397398
status: "ok",
398399
sampled: true,
399-
name: "a/path"
400+
name: "a/path",
401+
hub: Sentry.get_current_hub
400402
)
401403
end
402404

sentry-ruby/spec/sentry/scope_spec.rb

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@
77
new_breadcrumb
88
end
99

10+
let(:configuration) { Sentry::Configuration.new }
11+
let(:client) { Sentry::Client.new(configuration) }
12+
let(:hub) do
13+
Sentry::Hub.new(client, subject)
14+
end
15+
1016
describe "#initialize" do
1117
it "contains correct defaults" do
1218
expect(subject.breadcrumbs).to be_a(Sentry::BreadcrumbBuffer)
@@ -51,7 +57,7 @@
5157
it "deep-copies span as well" do
5258
perform_basic_setup
5359

54-
span = Sentry::Transaction.new(sampled: true)
60+
span = Sentry::Transaction.new(sampled: true, hub: hub)
5561
subject.set_span(span)
5662
copy = subject.dup
5763

@@ -138,7 +144,7 @@
138144
end
139145

140146
let(:transaction) do
141-
Sentry::Transaction.new(op: "parent")
147+
Sentry::Transaction.new(op: "parent", hub: hub)
142148
end
143149

144150
context "with span in the scope" do

sentry-ruby/spec/sentry/span_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@
103103
context "when the parent span has a span_recorder" do
104104
subject do
105105
# inherits the span recorder from the transaction
106-
Sentry::Transaction.new.start_child
106+
Sentry::Transaction.new(hub: Sentry.get_current_hub).start_child
107107
end
108108

109109
it "gives the child span its span_recorder" do
@@ -124,7 +124,7 @@
124124

125125
context "when the parent span has a transaction" do
126126
before do
127-
subject.transaction = Sentry::Transaction.new
127+
subject.transaction = Sentry::Transaction.new(hub: Sentry.get_current_hub)
128128
end
129129

130130
it "gives the child span its transaction" do

sentry-ruby/spec/sentry/transaction_spec.rb

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,11 @@
1212
status: "ok",
1313
sampled: true,
1414
parent_sampled: true,
15-
name: "foo"
15+
name: "foo",
16+
hub: Sentry.get_current_hub
1617
)
1718
end
1819

19-
describe "#initialize" do
20-
it "raises exception if there's no given hub and the SDK is not initialized" do
21-
allow(Sentry).to receive(:get_current_hub)
22-
23-
expect do
24-
described_class.new
25-
end.to raise_error(Sentry::Error)
26-
end
27-
end
28-
2920
describe ".from_sentry_trace" do
3021
let(:sentry_trace) { subject.to_sentry_trace }
3122

@@ -152,26 +143,26 @@
152143
it "sets @sampled to false and return" do
153144
allow(Sentry.configuration).to receive(:tracing_enabled?).and_return(false)
154145

155-
transaction = described_class.new(sampled: true)
146+
transaction = described_class.new(sampled: true, hub: Sentry.get_current_hub)
156147
transaction.set_initial_sample_decision(sampling_context: {})
157148
expect(transaction.sampled).to eq(false)
158149
end
159150
end
160151

161152
context "when tracing is enabled" do
162-
let(:subject) { described_class.new(op: "rack.request") }
153+
let(:subject) { described_class.new(op: "rack.request", hub: Sentry.get_current_hub) }
163154

164155
before do
165156
allow(Sentry.configuration).to receive(:tracing_enabled?).and_return(true)
166157
end
167158

168159
context "when the transaction already has a decision" do
169160
it "doesn't change it" do
170-
transaction = described_class.new(sampled: true)
161+
transaction = described_class.new(sampled: true, hub: Sentry.get_current_hub)
171162
transaction.set_initial_sample_decision(sampling_context: {})
172163
expect(transaction.sampled).to eq(true)
173164

174-
transaction = described_class.new(sampled: false)
165+
transaction = described_class.new(sampled: false, hub: Sentry.get_current_hub)
175166
transaction.set_initial_sample_decision(sampling_context: {})
176167
expect(transaction.sampled).to eq(false)
177168
end
@@ -255,17 +246,17 @@
255246
it "uses the genereted rate for sampling (positive)" do
256247
expect(Sentry.configuration.logger).to receive(:debug).exactly(3).and_call_original
257248

258-
subject = described_class.new
249+
subject = described_class.new(hub: Sentry.get_current_hub)
259250
Sentry.configuration.traces_sampler = -> (_) { true }
260251
subject.set_initial_sample_decision(sampling_context: {})
261252
expect(subject.sampled).to eq(true)
262253

263-
subject = described_class.new
254+
subject = described_class.new(hub: Sentry.get_current_hub)
264255
Sentry.configuration.traces_sampler = -> (_) { 1.0 }
265256
subject.set_initial_sample_decision(sampling_context: {})
266257
expect(subject.sampled).to eq(true)
267258

268-
subject = described_class.new
259+
subject = described_class.new(hub: Sentry.get_current_hub)
269260
Sentry.configuration.traces_sampler = -> (_) { 1 }
270261
subject.set_initial_sample_decision(sampling_context: {})
271262
expect(subject.sampled).to eq(true)
@@ -278,12 +269,12 @@
278269
it "uses the genereted rate for sampling (negative)" do
279270
expect(Sentry.configuration.logger).to receive(:debug).exactly(2).and_call_original
280271

281-
subject = described_class.new
272+
subject = described_class.new(hub: Sentry.get_current_hub)
282273
Sentry.configuration.traces_sampler = -> (_) { false }
283274
subject.set_initial_sample_decision(sampling_context: {})
284275
expect(subject.sampled).to eq(false)
285276

286-
subject = described_class.new
277+
subject = described_class.new(hub: Sentry.get_current_hub)
287278
Sentry.configuration.traces_sampler = -> (_) { 0.0 }
288279
subject.set_initial_sample_decision(sampling_context: {})
289280
expect(subject.sampled).to eq(false)
@@ -347,7 +338,7 @@
347338
end
348339

349340
context "if the transaction is not sampled" do
350-
subject { described_class.new(sampled: false) }
341+
subject { described_class.new(sampled: false, hub: Sentry.get_current_hub) }
351342

352343
it "doesn't send it" do
353344
subject.finish
@@ -357,7 +348,7 @@
357348
end
358349

359350
context "if the transaction doesn't have a name" do
360-
subject { described_class.new(sampled: true) }
351+
subject { described_class.new(sampled: true, hub: Sentry.get_current_hub) }
361352

362353
it "adds a default name" do
363354
subject.finish

0 commit comments

Comments
 (0)