Skip to content

Commit 87160d0

Browse files
herin049xrmx
herin049
and
xrmx
authored
fix: Instrument creation race condition (#4913)
* initial fix for instrumentation creation race condition * fix remaining instrument creation functions in SDK Meter * update CHANGELOG.md * fix lint error * update name of meter instrument lock --------- Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
1 parent 80bf955 commit 87160d0

File tree

3 files changed

+192
-137
lines changed

3 files changed

+192
-137
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4848
([#4891](https://github.com/open-telemetry/opentelemetry-python/pull/4891))
4949
- Implement experimental TracerConfigurator
5050
([#4861](https://github.com/open-telemetry/opentelemetry-python/pull/4861))
51+
- `opentelemetry-sdk`: Fix instrument creation race condition
52+
([#4913](https://github.com/open-telemetry/opentelemetry-python/pull/4913))
5153
- bump semantic-conventions to v1.39.0
5254
([#4914](https://github.com/open-telemetry/opentelemetry-python/pull/4914))
5355

‎opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py‎

Lines changed: 132 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,24 @@ def __init__(
8787
self._instrumentation_scope = instrumentation_scope
8888
self._measurement_consumer = measurement_consumer
8989
self._instrument_id_instrument = {}
90-
self._instrument_id_instrument_lock = Lock()
90+
self._instrument_registration_lock = Lock()
9191

9292
def create_counter(self, name, unit="", description="") -> APICounter:
93-
status = self._register_instrument(name, _Counter, unit, description)
93+
with self._instrument_registration_lock:
94+
status = self._register_instrument(
95+
name, _Counter, unit, description
96+
)
97+
if not status.already_registered:
98+
self._instrument_id_instrument[status.instrument_id] = (
99+
_Counter(
100+
name,
101+
self._instrumentation_scope,
102+
self._measurement_consumer,
103+
unit,
104+
description,
105+
)
106+
)
107+
instrument = self._instrument_id_instrument[status.instrument_id]
94108

95109
if status.conflict:
96110
# FIXME #2558 go through all views here and check if this
@@ -103,28 +117,26 @@ def create_counter(self, name, unit="", description="") -> APICounter:
103117
description,
104118
status,
105119
)
106-
if status.already_registered:
107-
with self._instrument_id_instrument_lock:
108-
return self._instrument_id_instrument[status.instrument_id]
109-
110-
instrument = _Counter(
111-
name,
112-
self._instrumentation_scope,
113-
self._measurement_consumer,
114-
unit,
115-
description,
116-
)
117-
118-
with self._instrument_id_instrument_lock:
119-
self._instrument_id_instrument[status.instrument_id] = instrument
120-
return instrument
120+
return instrument
121121

122122
def create_up_down_counter(
123123
self, name, unit="", description=""
124124
) -> APIUpDownCounter:
125-
status = self._register_instrument(
126-
name, _UpDownCounter, unit, description
127-
)
125+
with self._instrument_registration_lock:
126+
status = self._register_instrument(
127+
name, _UpDownCounter, unit, description
128+
)
129+
if not status.already_registered:
130+
self._instrument_id_instrument[status.instrument_id] = (
131+
_UpDownCounter(
132+
name,
133+
self._instrumentation_scope,
134+
self._measurement_consumer,
135+
unit,
136+
description,
137+
)
138+
)
139+
instrument = self._instrument_id_instrument[status.instrument_id]
128140

129141
if status.conflict:
130142
# FIXME #2558 go through all views here and check if this
@@ -137,21 +149,7 @@ def create_up_down_counter(
137149
description,
138150
status,
139151
)
140-
if status.already_registered:
141-
with self._instrument_id_instrument_lock:
142-
return self._instrument_id_instrument[status.instrument_id]
143-
144-
instrument = _UpDownCounter(
145-
name,
146-
self._instrumentation_scope,
147-
self._measurement_consumer,
148-
unit,
149-
description,
150-
)
151-
152-
with self._instrument_id_instrument_lock:
153-
self._instrument_id_instrument[status.instrument_id] = instrument
154-
return instrument
152+
return instrument
155153

156154
def create_observable_counter(
157155
self,
@@ -160,9 +158,27 @@ def create_observable_counter(
160158
unit="",
161159
description="",
162160
) -> APIObservableCounter:
163-
status = self._register_instrument(
164-
name, _ObservableCounter, unit, description
165-
)
161+
with self._instrument_registration_lock:
162+
status = self._register_instrument(
163+
name, _ObservableCounter, unit, description
164+
)
165+
if not status.already_registered:
166+
self._instrument_id_instrument[status.instrument_id] = (
167+
_ObservableCounter(
168+
name,
169+
self._instrumentation_scope,
170+
self._measurement_consumer,
171+
callbacks,
172+
unit,
173+
description,
174+
)
175+
)
176+
instrument = self._instrument_id_instrument[status.instrument_id]
177+
178+
if not status.already_registered:
179+
self._measurement_consumer.register_asynchronous_instrument(
180+
instrument
181+
)
166182

167183
if status.conflict:
168184
# FIXME #2558 go through all views here and check if this
@@ -175,24 +191,7 @@ def create_observable_counter(
175191
description,
176192
status,
177193
)
178-
if status.already_registered:
179-
with self._instrument_id_instrument_lock:
180-
return self._instrument_id_instrument[status.instrument_id]
181-
182-
instrument = _ObservableCounter(
183-
name,
184-
self._instrumentation_scope,
185-
self._measurement_consumer,
186-
callbacks,
187-
unit,
188-
description,
189-
)
190-
191-
self._measurement_consumer.register_asynchronous_instrument(instrument)
192-
193-
with self._instrument_id_instrument_lock:
194-
self._instrument_id_instrument[status.instrument_id] = instrument
195-
return instrument
194+
return instrument
196195

197196
def create_histogram(
198197
self,
@@ -223,13 +222,26 @@ def create_histogram(
223222
"explicit_bucket_boundaries_advisory must be a sequence of numbers"
224223
)
225224

226-
status = self._register_instrument(
227-
name,
228-
_Histogram,
229-
unit,
230-
description,
231-
explicit_bucket_boundaries_advisory,
232-
)
225+
with self._instrument_registration_lock:
226+
status = self._register_instrument(
227+
name,
228+
_Histogram,
229+
unit,
230+
description,
231+
explicit_bucket_boundaries_advisory,
232+
)
233+
if not status.already_registered:
234+
self._instrument_id_instrument[status.instrument_id] = (
235+
_Histogram(
236+
name,
237+
self._instrumentation_scope,
238+
self._measurement_consumer,
239+
unit,
240+
description,
241+
explicit_bucket_boundaries_advisory,
242+
)
243+
)
244+
instrument = self._instrument_id_instrument[status.instrument_id]
233245

234246
if status.conflict:
235247
# FIXME #2558 go through all views here and check if this
@@ -242,24 +254,20 @@ def create_histogram(
242254
description,
243255
status,
244256
)
245-
if status.already_registered:
246-
with self._instrument_id_instrument_lock:
247-
return self._instrument_id_instrument[status.instrument_id]
248-
249-
instrument = _Histogram(
250-
name,
251-
self._instrumentation_scope,
252-
self._measurement_consumer,
253-
unit,
254-
description,
255-
explicit_bucket_boundaries_advisory,
256-
)
257-
with self._instrument_id_instrument_lock:
258-
self._instrument_id_instrument[status.instrument_id] = instrument
259-
return instrument
257+
return instrument
260258

261259
def create_gauge(self, name, unit="", description="") -> APIGauge:
262-
status = self._register_instrument(name, _Gauge, unit, description)
260+
with self._instrument_registration_lock:
261+
status = self._register_instrument(name, _Gauge, unit, description)
262+
if not status.already_registered:
263+
self._instrument_id_instrument[status.instrument_id] = _Gauge(
264+
name,
265+
self._instrumentation_scope,
266+
self._measurement_consumer,
267+
unit,
268+
description,
269+
)
270+
instrument = self._instrument_id_instrument[status.instrument_id]
263271

264272
if status.conflict:
265273
# FIXME #2558 go through all views here and check if this
@@ -272,28 +280,32 @@ def create_gauge(self, name, unit="", description="") -> APIGauge:
272280
description,
273281
status,
274282
)
275-
if status.already_registered:
276-
with self._instrument_id_instrument_lock:
277-
return self._instrument_id_instrument[status.instrument_id]
278-
279-
instrument = _Gauge(
280-
name,
281-
self._instrumentation_scope,
282-
self._measurement_consumer,
283-
unit,
284-
description,
285-
)
286-
287-
with self._instrument_id_instrument_lock:
288-
self._instrument_id_instrument[status.instrument_id] = instrument
289-
return instrument
283+
return instrument
290284

291285
def create_observable_gauge(
292286
self, name, callbacks=None, unit="", description=""
293287
) -> APIObservableGauge:
294-
status = self._register_instrument(
295-
name, _ObservableGauge, unit, description
296-
)
288+
with self._instrument_registration_lock:
289+
status = self._register_instrument(
290+
name, _ObservableGauge, unit, description
291+
)
292+
if not status.already_registered:
293+
self._instrument_id_instrument[status.instrument_id] = (
294+
_ObservableGauge(
295+
name,
296+
self._instrumentation_scope,
297+
self._measurement_consumer,
298+
callbacks,
299+
unit,
300+
description,
301+
)
302+
)
303+
instrument = self._instrument_id_instrument[status.instrument_id]
304+
305+
if not status.already_registered:
306+
self._measurement_consumer.register_asynchronous_instrument(
307+
instrument
308+
)
297309

298310
if status.conflict:
299311
# FIXME #2558 go through all views here and check if this
@@ -306,31 +318,32 @@ def create_observable_gauge(
306318
description,
307319
status,
308320
)
309-
if status.already_registered:
310-
with self._instrument_id_instrument_lock:
311-
return self._instrument_id_instrument[status.instrument_id]
312-
313-
instrument = _ObservableGauge(
314-
name,
315-
self._instrumentation_scope,
316-
self._measurement_consumer,
317-
callbacks,
318-
unit,
319-
description,
320-
)
321-
322-
self._measurement_consumer.register_asynchronous_instrument(instrument)
323-
324-
with self._instrument_id_instrument_lock:
325-
self._instrument_id_instrument[status.instrument_id] = instrument
326-
return instrument
321+
return instrument
327322

328323
def create_observable_up_down_counter(
329324
self, name, callbacks=None, unit="", description=""
330325
) -> APIObservableUpDownCounter:
331-
status = self._register_instrument(
332-
name, _ObservableUpDownCounter, unit, description
333-
)
326+
with self._instrument_registration_lock:
327+
status = self._register_instrument(
328+
name, _ObservableUpDownCounter, unit, description
329+
)
330+
if not status.already_registered:
331+
self._instrument_id_instrument[status.instrument_id] = (
332+
_ObservableUpDownCounter(
333+
name,
334+
self._instrumentation_scope,
335+
self._measurement_consumer,
336+
callbacks,
337+
unit,
338+
description,
339+
)
340+
)
341+
instrument = self._instrument_id_instrument[status.instrument_id]
342+
343+
if not status.already_registered:
344+
self._measurement_consumer.register_asynchronous_instrument(
345+
instrument
346+
)
334347

335348
if status.conflict:
336349
# FIXME #2558 go through all views here and check if this
@@ -343,24 +356,7 @@ def create_observable_up_down_counter(
343356
description,
344357
status,
345358
)
346-
if status.already_registered:
347-
with self._instrument_id_instrument_lock:
348-
return self._instrument_id_instrument[status.instrument_id]
349-
350-
instrument = _ObservableUpDownCounter(
351-
name,
352-
self._instrumentation_scope,
353-
self._measurement_consumer,
354-
callbacks,
355-
unit,
356-
description,
357-
)
358-
359-
self._measurement_consumer.register_asynchronous_instrument(instrument)
360-
361-
with self._instrument_id_instrument_lock:
362-
self._instrument_id_instrument[status.instrument_id] = instrument
363-
return instrument
359+
return instrument
364360

365361

366362
def _get_exemplar_filter(exemplar_filter: str) -> ExemplarFilter:

0 commit comments

Comments
 (0)