Skip to content

Commit

Permalink
Fix: filter exemplar for observable instrument and export of exemplar…
Browse files Browse the repository at this point in the history
… without trace and span ids (#4251)

* Deal with missing span and trace ids

* Fix applying exemplar filter to observable instruments

* Lint the code

* add tests

* Add entry in changelog

* Fix span and trace id typing

* Fix CI

* Test consume_measurement is called for async instrument

* Add integration tests

* fix integration tests

Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com>

* Update opentelemetry-sdk/tests/metrics/integration_test/test_exemplars.py

* add test that default exemplar filter with no span does not create exemplar

---------

Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com>
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com>
Co-authored-by: Aaron Abbott <aaronabbott@google.com>
  • Loading branch information
5 people authored Nov 8, 2024
1 parent a2d77a0 commit c4fa8d7
Show file tree
Hide file tree
Showing 8 changed files with 532 additions and 29 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- Fix metrics export with exemplar and no context and filtering observable instruments
([#4251](https://github.com/open-telemetry/opentelemetry-python/pull/4251))

## Version 1.28.0/0.49b0 (2024-11-05)

- Removed superfluous py.typed markers and added them where they were missing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.
import logging
from os import environ
from typing import Dict
from typing import Dict, List

from opentelemetry.exporter.otlp.proto.common._internal import (
_encode_attributes,
Expand All @@ -34,6 +34,7 @@
)
from opentelemetry.sdk.metrics import (
Counter,
Exemplar,
Histogram,
ObservableCounter,
ObservableGauge,
Expand Down Expand Up @@ -341,7 +342,7 @@ def _encode_metric(metric, pb2_metric):
)


def _encode_exemplars(sdk_exemplars: list) -> list:
def _encode_exemplars(sdk_exemplars: List[Exemplar]) -> List[pb2.Exemplar]:
"""
Converts a list of SDK Exemplars into a list of protobuf Exemplars.
Expand All @@ -353,14 +354,26 @@ def _encode_exemplars(sdk_exemplars: list) -> list:
"""
pb_exemplars = []
for sdk_exemplar in sdk_exemplars:
pb_exemplar = pb2.Exemplar(
time_unix_nano=sdk_exemplar.time_unix_nano,
span_id=_encode_span_id(sdk_exemplar.span_id),
trace_id=_encode_trace_id(sdk_exemplar.trace_id),
filtered_attributes=_encode_attributes(
sdk_exemplar.filtered_attributes
),
)
if (
sdk_exemplar.span_id is not None
and sdk_exemplar.trace_id is not None
):
pb_exemplar = pb2.Exemplar(
time_unix_nano=sdk_exemplar.time_unix_nano,
span_id=_encode_span_id(sdk_exemplar.span_id),
trace_id=_encode_trace_id(sdk_exemplar.trace_id),
filtered_attributes=_encode_attributes(
sdk_exemplar.filtered_attributes
),
)
else:
pb_exemplar = pb2.Exemplar(
time_unix_nano=sdk_exemplar.time_unix_nano,
filtered_attributes=_encode_attributes(
sdk_exemplar.filtered_attributes
),
)

# Assign the value based on its type in the SDK exemplar
if isinstance(sdk_exemplar.value, float):
pb_exemplar.as_double = sdk_exemplar.value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# pylint: disable=protected-access
# pylint: disable=protected-access,too-many-lines
import unittest

from opentelemetry.exporter.otlp.proto.common._internal.metrics_encoder import (
Expand All @@ -33,6 +33,7 @@
from opentelemetry.proto.resource.v1.resource_pb2 import (
Resource as OTLPResource,
)
from opentelemetry.sdk.metrics import Exemplar
from opentelemetry.sdk.metrics.export import (
AggregationTemporality,
Buckets,
Expand All @@ -55,6 +56,9 @@


class TestOTLPMetricsEncoder(unittest.TestCase):
span_id = int("6e0c63257de34c92", 16)
trace_id = int("d4cda95b652f4a1592b449d5929fda1b", 16)

histogram = Metric(
name="histogram",
description="foo",
Expand All @@ -65,6 +69,22 @@ class TestOTLPMetricsEncoder(unittest.TestCase):
attributes={"a": 1, "b": True},
start_time_unix_nano=1641946016139533244,
time_unix_nano=1641946016139533244,
exemplars=[
Exemplar(
{"filtered": "banana"},
298.0,
1641946016139533400,
span_id,
trace_id,
),
Exemplar(
{"filtered": "banana"},
298.0,
1641946016139533400,
None,
None,
),
],
count=5,
sum=67,
bucket_counts=[1, 4],
Expand Down Expand Up @@ -460,7 +480,34 @@ def test_encode_histogram(self):
sum=67,
bucket_counts=[1, 4],
explicit_bounds=[10.0, 20.0],
exemplars=[],
exemplars=[
pb2.Exemplar(
time_unix_nano=1641946016139533400,
as_double=298,
span_id=b"n\x0cc%}\xe3L\x92",
trace_id=b"\xd4\xcd\xa9[e/J\x15\x92\xb4I\xd5\x92\x9f\xda\x1b",
filtered_attributes=[
KeyValue(
key="filtered",
value=AnyValue(
string_value="banana"
),
)
],
),
pb2.Exemplar(
time_unix_nano=1641946016139533400,
as_double=298,
filtered_attributes=[
KeyValue(
key="filtered",
value=AnyValue(
string_value="banana"
),
)
],
),
],
max=18.0,
min=8.0,
)
Expand Down Expand Up @@ -563,7 +610,34 @@ def test_encode_multiple_scope_histogram(self):
sum=67,
bucket_counts=[1, 4],
explicit_bounds=[10.0, 20.0],
exemplars=[],
exemplars=[
pb2.Exemplar(
time_unix_nano=1641946016139533400,
as_double=298,
span_id=b"n\x0cc%}\xe3L\x92",
trace_id=b"\xd4\xcd\xa9[e/J\x15\x92\xb4I\xd5\x92\x9f\xda\x1b",
filtered_attributes=[
KeyValue(
key="filtered",
value=AnyValue(
string_value="banana"
),
)
],
),
pb2.Exemplar(
time_unix_nano=1641946016139533400,
as_double=298,
filtered_attributes=[
KeyValue(
key="filtered",
value=AnyValue(
string_value="banana"
),
)
],
),
],
max=18.0,
min=8.0,
)
Expand Down Expand Up @@ -598,7 +672,34 @@ def test_encode_multiple_scope_histogram(self):
sum=67,
bucket_counts=[1, 4],
explicit_bounds=[10.0, 20.0],
exemplars=[],
exemplars=[
pb2.Exemplar(
time_unix_nano=1641946016139533400,
as_double=298,
span_id=b"n\x0cc%}\xe3L\x92",
trace_id=b"\xd4\xcd\xa9[e/J\x15\x92\xb4I\xd5\x92\x9f\xda\x1b",
filtered_attributes=[
KeyValue(
key="filtered",
value=AnyValue(
string_value="banana"
),
)
],
),
pb2.Exemplar(
time_unix_nano=1641946016139533400,
as_double=298,
filtered_attributes=[
KeyValue(
key="filtered",
value=AnyValue(
string_value="banana"
),
)
],
),
],
max=18.0,
min=8.0,
)
Expand Down Expand Up @@ -640,7 +741,34 @@ def test_encode_multiple_scope_histogram(self):
sum=67,
bucket_counts=[1, 4],
explicit_bounds=[10.0, 20.0],
exemplars=[],
exemplars=[
pb2.Exemplar(
time_unix_nano=1641946016139533400,
as_double=298,
span_id=b"n\x0cc%}\xe3L\x92",
trace_id=b"\xd4\xcd\xa9[e/J\x15\x92\xb4I\xd5\x92\x9f\xda\x1b",
filtered_attributes=[
KeyValue(
key="filtered",
value=AnyValue(
string_value="banana"
),
)
],
),
pb2.Exemplar(
time_unix_nano=1641946016139533400,
as_double=298,
filtered_attributes=[
KeyValue(
key="filtered",
value=AnyValue(
string_value="banana"
),
)
],
),
],
max=18.0,
min=8.0,
)
Expand Down Expand Up @@ -682,7 +810,34 @@ def test_encode_multiple_scope_histogram(self):
sum=67,
bucket_counts=[1, 4],
explicit_bounds=[10.0, 20.0],
exemplars=[],
exemplars=[
pb2.Exemplar(
time_unix_nano=1641946016139533400,
as_double=298,
span_id=b"n\x0cc%}\xe3L\x92",
trace_id=b"\xd4\xcd\xa9[e/J\x15\x92\xb4I\xd5\x92\x9f\xda\x1b",
filtered_attributes=[
KeyValue(
key="filtered",
value=AnyValue(
string_value="banana"
),
)
],
),
pb2.Exemplar(
time_unix_nano=1641946016139533400,
as_double=298,
filtered_attributes=[
KeyValue(
key="filtered",
value=AnyValue(
string_value="banana"
),
)
],
),
],
max=18.0,
min=8.0,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,5 @@ class Exemplar:
filtered_attributes: Attributes
value: Union[int, float]
time_unix_nano: int
span_id: Optional[str] = None
trace_id: Optional[str] = None
span_id: Optional[int] = None
trace_id: Optional[int] = None
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ def __init__(self) -> None:
self.__value: Union[int, float] = 0
self.__attributes: Attributes = None
self.__time_unix_nano: int = 0
self.__span_id: Optional[str] = None
self.__trace_id: Optional[str] = None
self.__span_id: Optional[int] = None
self.__trace_id: Optional[int] = None
self.__offered: bool = False

def offer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,17 @@ def __init__(
] = []

def consume_measurement(self, measurement: Measurement) -> None:
should_sample_exemplar = (
self._sdk_config.exemplar_filter.should_sample(
measurement.value,
measurement.time_unix_nano,
measurement.attributes,
measurement.context,
)
)
for reader_storage in self._reader_storages.values():
reader_storage.consume_measurement(
measurement,
self._sdk_config.exemplar_filter.should_sample(
measurement.value,
measurement.time_unix_nano,
measurement.attributes,
measurement.context,
),
measurement, should_sample_exemplar
)

def register_asynchronous_instrument(
Expand Down Expand Up @@ -126,7 +128,17 @@ def collect(
)

for measurement in measurements:
metric_reader_storage.consume_measurement(measurement)
should_sample_exemplar = (
self._sdk_config.exemplar_filter.should_sample(
measurement.value,
measurement.time_unix_nano,
measurement.attributes,
measurement.context,
)
)
metric_reader_storage.consume_measurement(
measurement, should_sample_exemplar
)

result = self._reader_storages[metric_reader].collect()

Expand Down
Loading

0 comments on commit c4fa8d7

Please sign in to comment.