Skip to content

Commit 28a6af8

Browse files
zvkempzvkemp
committed
fix test state reset issue
1 parent c65742e commit 28a6af8

File tree

4 files changed

+120
-38
lines changed

4 files changed

+120
-38
lines changed

‎instrumentation/base/lib/opentelemetry/instrumentation/base.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -390,9 +390,9 @@ def metrics_enabled_by_env_var?
390390
var_name.upcase!
391391
var_name.gsub!('::', '_')
392392
var_name.gsub!('OPENTELEMETRY_', 'OTEL_RUBY_')
393-
var_name << 'METRICS_ENABLED'
393+
var_name << '_METRICS_ENABLED'
394394

395-
ENV[var_name] != 'false'
395+
ENV.key?(var_name) && ENV[var_name] != 'false'
396396
end
397397

398398
# Checks to see if the user has passed any environment variables that set options

‎instrumentation/sidekiq/test/opentelemetry/instrumentation/sidekiq/middlewares/client/tracer_middleware_test.rb

+39
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,20 @@
1414
let(:spans) { exporter.finished_spans }
1515
let(:enqueue_span) { spans.first }
1616
let(:config) { {} }
17+
let(:metrics_exporter) { METRICS_EXPORTER }
18+
19+
with_metrics_sdk do
20+
let(:metric_snapshots) do
21+
METRICS_EXPORTER.tap(&:pull)
22+
.metric_snapshots.select { |snapshot| snapshot.data_points.any? }
23+
.group_by(&:name)
24+
end
25+
end
1726

1827
before do
1928
instrumentation.install(config)
2029
exporter.reset
30+
reset_metrics_exporter
2131
end
2232

2333
after do
@@ -81,5 +91,34 @@
8191
_(enqueue_span.attributes['peer.service']).must_equal 'MySidekiqService'
8292
end
8393
end
94+
95+
with_metrics_sdk do
96+
it 'yields no metrics if config is not set' do
97+
_(instrumentation.metrics_enabled?).must_equal false
98+
SimpleJob.perform_async
99+
SimpleJob.drain
100+
101+
_(metric_snapshots).must_be_empty
102+
end
103+
104+
describe 'with metrics enabled' do
105+
let(:config) { { metrics: true } }
106+
107+
it 'metrics processing' do
108+
_(instrumentation.metrics_enabled?).must_equal true
109+
SimpleJob.perform_async
110+
SimpleJob.drain
111+
112+
sent_messages = metric_snapshots['messaging.client.sent.messages']
113+
_(sent_messages.count).must_equal 1
114+
_(sent_messages.first.data_points.count).must_equal 1
115+
_(sent_messages.first.data_points.first.value).must_equal 1
116+
sent_messages_attributes = sent_messages.first.data_points.first.attributes
117+
_(sent_messages_attributes['messaging.system']).must_equal 'sidekiq'
118+
_(sent_messages_attributes['messaging.destination.name']).must_equal 'default' # FIXME: newer semconv specifies this key
119+
_(sent_messages_attributes['messaging.operation.name']).must_equal 'create'
120+
end
121+
end
122+
end
84123
end
85124
end

‎instrumentation/sidekiq/test/opentelemetry/instrumentation/sidekiq/middlewares/server/tracer_middleware_test.rb

+50-32
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,24 @@
1111
describe OpenTelemetry::Instrumentation::Sidekiq::Middlewares::Server::TracerMiddleware do
1212
let(:instrumentation) { OpenTelemetry::Instrumentation::Sidekiq::Instrumentation.instance }
1313
let(:exporter) { EXPORTER }
14-
let(:metrics_exporter) { METRICS_EXPORTER }
1514
let(:spans) { exporter.finished_spans }
1615
let(:enqueuer_span) { spans.first }
1716
let(:job_span) { spans.last }
1817
let(:root_span) { spans.find { |s| s.parent_span_id == OpenTelemetry::Trace::INVALID_SPAN_ID } }
1918
let(:config) { {} }
2019

21-
with_metrics do
20+
with_metrics_sdk do
2221
let(:metric_snapshots) do
23-
METRICS_EXPORTER.tap(&:pull).metric_snapshots.group_by(&:name)
22+
METRICS_EXPORTER.tap(&:pull)
23+
.metric_snapshots.select { |snapshot| snapshot.data_points.any? }
24+
.group_by(&:name)
2425
end
2526
end
2627

2728
before do
2829
instrumentation.install(config)
2930
exporter.reset
30-
with_metrics { metrics_exporter.tap(&:pull).reset }
31+
reset_metrics_exporter
3132
end
3233

3334
after do
@@ -59,35 +60,52 @@
5960
_(job_span.events[1].name).must_equal('enqueued_at')
6061
end
6162

62-
it 'metrics processing', with_metrics: true do
63-
job_id = SimpleJob.perform_async
64-
SimpleJob.drain
63+
with_metrics_sdk do
64+
# FIXME: still seeing order-dependent failure here
65+
it 'yields no metrics if config is not set' do
66+
_(OpenTelemetry::Instrumentation::Sidekiq::Instrumentation.instance.metrics_enabled?).must_equal false
67+
job_id = SimpleJob.perform_async
68+
SimpleJob.drain
6569

66-
queue_latency = metric_snapshots['messaging.queue.latency']
67-
_(queue_latency.count).must_equal 1
68-
_(queue_latency.first.data_points.count).must_equal 1
69-
queue_latency_attributes = queue_latency.first.data_points.first.attributes
70-
_(queue_latency_attributes['messaging.system']).must_equal 'sidekiq'
71-
_(queue_latency_attributes['messaging.destination.name']).must_equal 'default' # FIXME: newer semconv specifies this key
72-
73-
process_duration = metric_snapshots['messaging.process.duration']
74-
_(process_duration.count).must_equal 1
75-
_(process_duration.first.data_points.count).must_equal 1
76-
process_duration_attributes = process_duration.first.data_points.first.attributes
77-
_(process_duration_attributes['messaging.system']).must_equal 'sidekiq'
78-
_(process_duration_attributes['messaging.operation.name']).must_equal 'process'
79-
_(process_duration_attributes['messaging.destination.name']).must_equal 'default'
80-
81-
process_duration_data_point = process_duration.first.data_points.first
82-
_(process_duration_data_point.count).must_equal 1
83-
84-
consumed_messages = metric_snapshots['messaging.client.consumed.messages']
85-
_(consumed_messages.count).must_equal 1
86-
_(consumed_messages.first.data_points.count).must_equal 1
87-
consumed_messages_attributes = queue_latency.first.data_points.first.attributes
88-
_(consumed_messages_attributes['messaging.system']).must_equal 'sidekiq'
89-
_(consumed_messages_attributes['messaging.destination.name']).must_equal 'default' # FIXME: newer semconv specifies this key
90-
_(consumed_messages.first.data_points.first.value).must_equal 1
70+
_(exporter.finished_spans.size).must_equal 2
71+
_(metric_snapshots).must_be_empty
72+
end
73+
74+
describe 'with metrics enabled' do
75+
let(:config) { { metrics: true } }
76+
77+
it 'metrics processing' do
78+
_(OpenTelemetry::Instrumentation::Sidekiq::Instrumentation.instance.metrics_enabled?).must_equal true
79+
job_id = SimpleJob.perform_async
80+
SimpleJob.drain
81+
82+
queue_latency = metric_snapshots['messaging.queue.latency']
83+
_(queue_latency.count).must_equal 1
84+
_(queue_latency.first.data_points.count).must_equal 1
85+
queue_latency_attributes = queue_latency.first.data_points.first.attributes
86+
_(queue_latency_attributes['messaging.system']).must_equal 'sidekiq'
87+
_(queue_latency_attributes['messaging.destination.name']).must_equal 'default' # FIXME: newer semconv specifies this key
88+
89+
process_duration = metric_snapshots['messaging.process.duration']
90+
_(process_duration.count).must_equal 1
91+
_(process_duration.first.data_points.count).must_equal 1
92+
process_duration_attributes = process_duration.first.data_points.first.attributes
93+
_(process_duration_attributes['messaging.system']).must_equal 'sidekiq'
94+
_(process_duration_attributes['messaging.operation.name']).must_equal 'process'
95+
_(process_duration_attributes['messaging.destination.name']).must_equal 'default'
96+
97+
process_duration_data_point = process_duration.first.data_points.first
98+
_(process_duration_data_point.count).must_equal 1
99+
100+
consumed_messages = metric_snapshots['messaging.client.consumed.messages']
101+
_(consumed_messages.count).must_equal 1
102+
_(consumed_messages.first.data_points.count).must_equal 1
103+
consumed_messages_attributes = queue_latency.first.data_points.first.attributes
104+
_(consumed_messages_attributes['messaging.system']).must_equal 'sidekiq'
105+
_(consumed_messages_attributes['messaging.destination.name']).must_equal 'default' # FIXME: newer semconv specifies this key
106+
_(consumed_messages.first.data_points.first.value).must_equal 1
107+
end
108+
end
91109
end
92110

93111
it 'traces when enqueued with Active Job' do

‎instrumentation/sidekiq/test/test_helper.rb

+29-4
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,22 @@ def sdk_loaded?
5656
end
5757
end
5858

59+
# NOTE: this isn't currently used, but it may be useful to fully reset state between tests
60+
def reset_meter_provider
61+
return unless LoadedMetricsFeatures.sdk_loaded?
62+
63+
resource = OpenTelemetry.meter_provider.resource
64+
OpenTelemetry.meter_provider = OpenTelemetry::SDK::Metrics::MeterProvider.new(resource: resource)
65+
OpenTelemetry.meter_provider.add_metric_reader(METRICS_EXPORTER)
66+
end
67+
68+
def reset_metrics_exporter
69+
return unless LoadedMetricsFeatures.sdk_loaded?
70+
71+
METRICS_EXPORTER.pull
72+
METRICS_EXPORTER.reset
73+
end
74+
5975
if LoadedMetricsFeatures.sdk_loaded?
6076
METRICS_EXPORTER = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new
6177
OpenTelemetry.meter_provider.add_metric_reader(METRICS_EXPORTER)
@@ -70,13 +86,22 @@ def self.prepended(base)
7086
base.extend(self)
7187
end
7288

73-
def with_metrics
89+
def with_metrics_sdk
7490
yield if LoadedMetricsFeatures.sdk_loaded?
7591
end
7692

77-
def it(desc = 'anonymous', with_metrics: false, &block)
78-
return super(desc, &block) unless with_metrics
79-
return unless LoadedMetricsFeatures.sdk_loaded?
93+
# FIXME: unclear if this is ever needed
94+
def without_metrics_sdk
95+
yield unless LoadedMetricsFeatures.sdk_loaded?
96+
end
97+
98+
def it(desc = 'anonymous', with_metrics_sdk: false, without_metrics_sdk: false, &block)
99+
return super(desc, &block) unless with_metrics_sdk || without_metrics_sdk
100+
101+
raise ArgumentError, 'without_metrics_sdk and with_metrics_sdk must be mutually exclusive' if without_metrics_sdk && with_metrics_sdk
102+
103+
return if with_metrics_sdk && !LoadedMetricsFeatures.sdk_loaded?
104+
return if without_metrics_sdk && LoadedMetricsFeatures.sdk_loaded?
80105

81106
super(desc, &block)
82107
end

0 commit comments

Comments
 (0)