Skip to content

Commit 0de0eab

Browse files
ocelotlcodeboten
ocelotl
and
codeboten
authored
Aggregate together data from identical instruments (#2603)
* Aggregate together data from identical instruments Fixes #2557 * Remove duplicate test * Update boolean value * Update opentelemetry-api/src/opentelemetry/_metrics/__init__.py Co-authored-by: Alex Boten <alex@boten.ca> * Add sleep Co-authored-by: Alex Boten <alex@boten.ca>
1 parent acaef96 commit 0de0eab

File tree

3 files changed

+205
-33
lines changed

3 files changed

+205
-33
lines changed

‎opentelemetry-api/src/opentelemetry/_metrics/__init__.py‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
from logging import getLogger
4747
from os import environ
4848
from threading import Lock
49-
from typing import List, Optional, Set, cast
49+
from typing import List, Optional, Set, Tuple, cast
5050

5151
from opentelemetry._metrics.instrument import (
5252
Counter,
@@ -197,27 +197,31 @@ def schema_url(self):
197197
"""
198198
return self._schema_url
199199

200-
def _check_instrument_id(
200+
def _is_instrument_registered(
201201
self, name: str, type_: type, unit: str, description: str
202-
) -> bool:
202+
) -> Tuple[bool, str]:
203203
"""
204204
Check if an instrument with the same name, type, unit and description
205205
has been registered already.
206-
"""
207206
208-
result = False
207+
Returns a tuple. The first value is `True` if the instrument has been
208+
registered already, `False` otherwise. The second value is the
209+
instrument id.
210+
"""
209211

210212
instrument_id = ",".join(
211213
[name.strip().lower(), type_.__name__, unit, description]
212214
)
213215

216+
result = False
217+
214218
with self._instrument_ids_lock:
215219
if instrument_id in self._instrument_ids:
216220
result = True
217221
else:
218222
self._instrument_ids.add(instrument_id)
219223

220-
return result
224+
return (result, instrument_id)
221225

222226
@abstractmethod
223227
def create_counter(
@@ -489,7 +493,9 @@ class NoOpMeter(Meter):
489493
def create_counter(self, name, unit="", description="") -> Counter:
490494
"""Returns a no-op Counter."""
491495
super().create_counter(name, unit=unit, description=description)
492-
if self._check_instrument_id(name, NoOpCounter, unit, description):
496+
if self._is_instrument_registered(
497+
name, NoOpCounter, unit, description
498+
)[0]:
493499
_logger.warning(
494500
"An instrument with name %s, type %s, unit %s and "
495501
"description %s has been created already.",
@@ -507,9 +513,9 @@ def create_up_down_counter(
507513
super().create_up_down_counter(
508514
name, unit=unit, description=description
509515
)
510-
if self._check_instrument_id(
516+
if self._is_instrument_registered(
511517
name, NoOpUpDownCounter, unit, description
512-
):
518+
)[0]:
513519
_logger.warning(
514520
"An instrument with name %s, type %s, unit %s and "
515521
"description %s has been created already.",
@@ -527,9 +533,9 @@ def create_observable_counter(
527533
super().create_observable_counter(
528534
name, callbacks, unit=unit, description=description
529535
)
530-
if self._check_instrument_id(
536+
if self._is_instrument_registered(
531537
name, NoOpObservableCounter, unit, description
532-
):
538+
)[0]:
533539
_logger.warning(
534540
"An instrument with name %s, type %s, unit %s and "
535541
"description %s has been created already.",
@@ -548,7 +554,9 @@ def create_observable_counter(
548554
def create_histogram(self, name, unit="", description="") -> Histogram:
549555
"""Returns a no-op Histogram."""
550556
super().create_histogram(name, unit=unit, description=description)
551-
if self._check_instrument_id(name, NoOpHistogram, unit, description):
557+
if self._is_instrument_registered(
558+
name, NoOpHistogram, unit, description
559+
)[0]:
552560
_logger.warning(
553561
"An instrument with name %s, type %s, unit %s and "
554562
"description %s has been created already.",
@@ -566,9 +574,9 @@ def create_observable_gauge(
566574
super().create_observable_gauge(
567575
name, callbacks, unit=unit, description=description
568576
)
569-
if self._check_instrument_id(
577+
if self._is_instrument_registered(
570578
name, NoOpObservableGauge, unit, description
571-
):
579+
)[0]:
572580
_logger.warning(
573581
"An instrument with name %s, type %s, unit %s and "
574582
"description %s has been created already.",
@@ -591,9 +599,9 @@ def create_observable_up_down_counter(
591599
super().create_observable_up_down_counter(
592600
name, callbacks, unit=unit, description=description
593601
)
594-
if self._check_instrument_id(
602+
if self._is_instrument_registered(
595603
name, NoOpObservableUpDownCounter, unit, description
596-
):
604+
)[0]:
597605
_logger.warning(
598606
"An instrument with name %s, type %s, unit %s and "
599607
"description %s has been created already.",

‎opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py‎

Lines changed: 86 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,17 @@ def __init__(
6363
super().__init__(instrumentation_scope)
6464
self._instrumentation_scope = instrumentation_scope
6565
self._measurement_consumer = measurement_consumer
66+
self._instrument_id_instrument = {}
67+
self._instrument_id_instrument_lock = Lock()
6668

6769
def create_counter(self, name, unit="", description="") -> APICounter:
68-
if self._check_instrument_id(name, Counter, unit, description):
70+
71+
(
72+
is_instrument_registered,
73+
instrument_id,
74+
) = self._is_instrument_registered(name, Counter, unit, description)
75+
76+
if is_instrument_registered:
6977
# FIXME #2558 go through all views here and check if this
7078
# instrument registration conflict can be fixed. If it can be, do
7179
# not log the following warning.
@@ -77,19 +85,33 @@ def create_counter(self, name, unit="", description="") -> APICounter:
7785
unit,
7886
description,
7987
)
88+
with self._instrument_id_instrument_lock:
89+
return self._instrument_id_instrument[instrument_id]
8090

81-
return Counter(
91+
instrument = Counter(
8292
name,
8393
self._instrumentation_scope,
8494
self._measurement_consumer,
8595
unit,
8696
description,
8797
)
8898

99+
with self._instrument_id_instrument_lock:
100+
self._instrument_id_instrument[instrument_id] = instrument
101+
return instrument
102+
89103
def create_up_down_counter(
90104
self, name, unit="", description=""
91105
) -> APIUpDownCounter:
92-
if self._check_instrument_id(name, UpDownCounter, unit, description):
106+
107+
(
108+
is_instrument_registered,
109+
instrument_id,
110+
) = self._is_instrument_registered(
111+
name, UpDownCounter, unit, description
112+
)
113+
114+
if is_instrument_registered:
93115
# FIXME #2558 go through all views here and check if this
94116
# instrument registration conflict can be fixed. If it can be, do
95117
# not log the following warning.
@@ -101,21 +123,33 @@ def create_up_down_counter(
101123
unit,
102124
description,
103125
)
126+
with self._instrument_id_instrument_lock:
127+
return self._instrument_id_instrument[instrument_id]
104128

105-
return UpDownCounter(
129+
instrument = UpDownCounter(
106130
name,
107131
self._instrumentation_scope,
108132
self._measurement_consumer,
109133
unit,
110134
description,
111135
)
112136

137+
with self._instrument_id_instrument_lock:
138+
self._instrument_id_instrument[instrument_id] = instrument
139+
return instrument
140+
113141
def create_observable_counter(
114142
self, name, callbacks=None, unit="", description=""
115143
) -> APIObservableCounter:
116-
if self._check_instrument_id(
144+
145+
(
146+
is_instrument_registered,
147+
instrument_id,
148+
) = self._is_instrument_registered(
117149
name, ObservableCounter, unit, description
118-
):
150+
)
151+
152+
if is_instrument_registered:
119153
# FIXME #2558 go through all views here and check if this
120154
# instrument registration conflict can be fixed. If it can be, do
121155
# not log the following warning.
@@ -127,6 +161,9 @@ def create_observable_counter(
127161
unit,
128162
description,
129163
)
164+
with self._instrument_id_instrument_lock:
165+
return self._instrument_id_instrument[instrument_id]
166+
130167
instrument = ObservableCounter(
131168
name,
132169
self._instrumentation_scope,
@@ -138,10 +175,18 @@ def create_observable_counter(
138175

139176
self._measurement_consumer.register_asynchronous_instrument(instrument)
140177

141-
return instrument
178+
with self._instrument_id_instrument_lock:
179+
self._instrument_id_instrument[instrument_id] = instrument
180+
return instrument
142181

143182
def create_histogram(self, name, unit="", description="") -> APIHistogram:
144-
if self._check_instrument_id(name, Histogram, unit, description):
183+
184+
(
185+
is_instrument_registered,
186+
instrument_id,
187+
) = self._is_instrument_registered(name, Histogram, unit, description)
188+
189+
if is_instrument_registered:
145190
# FIXME #2558 go through all views here and check if this
146191
# instrument registration conflict can be fixed. If it can be, do
147192
# not log the following warning.
@@ -153,18 +198,32 @@ def create_histogram(self, name, unit="", description="") -> APIHistogram:
153198
unit,
154199
description,
155200
)
156-
return Histogram(
201+
with self._instrument_id_instrument_lock:
202+
return self._instrument_id_instrument[instrument_id]
203+
204+
instrument = Histogram(
157205
name,
158206
self._instrumentation_scope,
159207
self._measurement_consumer,
160208
unit,
161209
description,
162210
)
211+
with self._instrument_id_instrument_lock:
212+
self._instrument_id_instrument[instrument_id] = instrument
213+
return instrument
163214

164215
def create_observable_gauge(
165216
self, name, callbacks=None, unit="", description=""
166217
) -> APIObservableGauge:
167-
if self._check_instrument_id(name, ObservableGauge, unit, description):
218+
219+
(
220+
is_instrument_registered,
221+
instrument_id,
222+
) = self._is_instrument_registered(
223+
name, ObservableGauge, unit, description
224+
)
225+
226+
if is_instrument_registered:
168227
# FIXME #2558 go through all views here and check if this
169228
# instrument registration conflict can be fixed. If it can be, do
170229
# not log the following warning.
@@ -176,6 +235,8 @@ def create_observable_gauge(
176235
unit,
177236
description,
178237
)
238+
with self._instrument_id_instrument_lock:
239+
return self._instrument_id_instrument[instrument_id]
179240

180241
instrument = ObservableGauge(
181242
name,
@@ -188,14 +249,20 @@ def create_observable_gauge(
188249

189250
self._measurement_consumer.register_asynchronous_instrument(instrument)
190251

191-
return instrument
252+
with self._instrument_id_instrument_lock:
253+
self._instrument_id_instrument[instrument_id] = instrument
254+
return instrument
192255

193256
def create_observable_up_down_counter(
194257
self, name, callbacks=None, unit="", description=""
195258
) -> APIObservableUpDownCounter:
196-
if self._check_instrument_id(
197-
name, ObservableUpDownCounter, unit, description
198-
):
259+
260+
(
261+
is_instrument_registered,
262+
instrument_id,
263+
) = self._is_instrument_registered(name, Counter, unit, description)
264+
265+
if is_instrument_registered:
199266
# FIXME #2558 go through all views here and check if this
200267
# instrument registration conflict can be fixed. If it can be, do
201268
# not log the following warning.
@@ -207,6 +274,8 @@ def create_observable_up_down_counter(
207274
unit,
208275
description,
209276
)
277+
with self._instrument_id_instrument_lock:
278+
return self._instrument_id_instrument[instrument_id]
210279

211280
instrument = ObservableUpDownCounter(
212281
name,
@@ -219,7 +288,9 @@ def create_observable_up_down_counter(
219288

220289
self._measurement_consumer.register_asynchronous_instrument(instrument)
221290

222-
return instrument
291+
with self._instrument_id_instrument_lock:
292+
self._instrument_id_instrument[instrument_id] = instrument
293+
return instrument
223294

224295

225296
class MeterProvider(APIMeterProvider):

0 commit comments

Comments
 (0)