From 43804f372149b2b7bafd90c880ecaee909d5ec3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Wed, 6 Nov 2024 18:05:34 -0300 Subject: [PATCH 1/6] fix workflows (#4256) * fix workflows Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> * remove changelog line since it's working Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> --------- Signed-off-by: emdneto <9735060+emdneto@users.noreply.github.com> --- .github/workflows/check-links.yml | 3 +-- .github/workflows/lint_0.yml | 2 +- .github/workflows/misc_0.yml | 4 +++- tox.ini | 1 + 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml index 770bd24168..9092cbaf78 100644 --- a/.github/workflows/check-links.yml +++ b/.github/workflows/check-links.yml @@ -23,7 +23,6 @@ jobs: check-links: runs-on: ubuntu-latest needs: changedfiles - if: if: | github.event.pull_request.user.login != 'opentelemetrybot' && github.event_name == 'pull_request' && ${{needs.changedfiles.outputs.md}} @@ -34,7 +33,7 @@ jobs: fetch-depth: 0 - name: Install markdown-link-check - run: npm install -g markdown-link-check + run: npm install -g markdown-link-check@v3.12.2 - name: Run markdown-link-check run: | diff --git a/.github/workflows/lint_0.yml b/.github/workflows/lint_0.yml index cd3cd7bda1..71b148c02d 100644 --- a/.github/workflows/lint_0.yml +++ b/.github/workflows/lint_0.yml @@ -11,7 +11,7 @@ on: env: CORE_REPO_SHA: main - CONTRIB_REPO_SHA: be0c78c8c11d05787edb53104c92a3a959f6a70c + CONTRIB_REPO_SHA: main PIP_EXISTS_ACTION: w jobs: diff --git a/.github/workflows/misc_0.yml b/.github/workflows/misc_0.yml index 3e8aa7cf1b..e816656b8d 100644 --- a/.github/workflows/misc_0.yml +++ b/.github/workflows/misc_0.yml @@ -11,7 +11,7 @@ on: env: CORE_REPO_SHA: main - CONTRIB_REPO_SHA: be0c78c8c11d05787edb53104c92a3a959f6a70c + CONTRIB_REPO_SHA: main PIP_EXISTS_ACTION: w jobs: @@ -109,6 +109,8 @@ jobs: docs: name: docs runs-on: ubuntu-latest + if: | + github.event.pull_request.user.login != 'opentelemetrybot' && github.event_name == 'pull_request' steps: - name: Checkout repo @ SHA - ${{ github.sha }} uses: actions/checkout@v4 diff --git a/tox.ini b/tox.ini index 49e2fc7710..3e92195f9d 100644 --- a/tox.ini +++ b/tox.ini @@ -334,6 +334,7 @@ commands = python {toxinidir}/scripts/public_symbols_checker.py [testenv:generate-workflows] +recreate = True deps = {env:CONTRIB_REPO}\#egg=generate_workflows_lib&subdirectory=.github/workflows/generate_workflows_lib commands = From 285893baf2be850a7cb58e94505a343ce5f910b0 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 7 Nov 2024 16:48:28 -0800 Subject: [PATCH 2/6] Regenerate workflows for contrib on core (#4258) --- .github/workflows/contrib_0.yml | 164 +++++++++++++++++++++++++++++--- 1 file changed, 152 insertions(+), 12 deletions(-) diff --git a/.github/workflows/contrib_0.yml b/.github/workflows/contrib_0.yml index 6a515027a7..ffd46c46d1 100644 --- a/.github/workflows/contrib_0.yml +++ b/.github/workflows/contrib_0.yml @@ -16,8 +16,8 @@ env: jobs: - py38-test-instrumentation-openai-v2: - name: instrumentation-openai-v2 + py38-test-instrumentation-openai-v2-0: + name: instrumentation-openai-v2-0 runs-on: ubuntu-latest steps: - name: Checkout contrib repo @ SHA - ${{ env.CONTRIB_REPO_SHA }} @@ -42,7 +42,35 @@ jobs: run: pip install tox - name: Run tests - run: tox -e py38-test-instrumentation-openai-v2 -- -ra + run: tox -e py38-test-instrumentation-openai-v2-0 -- -ra + + py38-test-instrumentation-openai-v2-1: + name: instrumentation-openai-v2-1 + runs-on: ubuntu-latest + steps: + - name: Checkout contrib repo @ SHA - ${{ env.CONTRIB_REPO_SHA }} + uses: actions/checkout@v4 + with: + repository: open-telemetry/opentelemetry-python-contrib + ref: ${{ env.CONTRIB_REPO_SHA }} + + - name: Checkout core repo @ SHA - ${{ github.sha }} + uses: actions/checkout@v4 + with: + repository: open-telemetry/opentelemetry-python + path: opentelemetry-python-core + + - name: Set up Python 3.8 + uses: actions/setup-python@v5 + with: + python-version: "3.8" + architecture: "x64" + + - name: Install tox + run: pip install tox + + - name: Run tests + run: tox -e py38-test-instrumentation-openai-v2-1 -- -ra py38-test-resource-detector-container: name: resource-detector-container @@ -72,8 +100,8 @@ jobs: - name: Run tests run: tox -e py38-test-resource-detector-container -- -ra - py38-test-resource-detector-azure: - name: resource-detector-azure + py38-test-resource-detector-azure-0: + name: resource-detector-azure-0 runs-on: ubuntu-latest steps: - name: Checkout contrib repo @ SHA - ${{ env.CONTRIB_REPO_SHA }} @@ -98,10 +126,10 @@ jobs: run: pip install tox - name: Run tests - run: tox -e py38-test-resource-detector-azure -- -ra + run: tox -e py38-test-resource-detector-azure-0 -- -ra - py38-test-sdk-extension-aws: - name: sdk-extension-aws + py38-test-resource-detector-azure-1: + name: resource-detector-azure-1 runs-on: ubuntu-latest steps: - name: Checkout contrib repo @ SHA - ${{ env.CONTRIB_REPO_SHA }} @@ -126,7 +154,63 @@ jobs: run: pip install tox - name: Run tests - run: tox -e py38-test-sdk-extension-aws -- -ra + run: tox -e py38-test-resource-detector-azure-1 -- -ra + + py38-test-sdk-extension-aws-0: + name: sdk-extension-aws-0 + runs-on: ubuntu-latest + steps: + - name: Checkout contrib repo @ SHA - ${{ env.CONTRIB_REPO_SHA }} + uses: actions/checkout@v4 + with: + repository: open-telemetry/opentelemetry-python-contrib + ref: ${{ env.CONTRIB_REPO_SHA }} + + - name: Checkout core repo @ SHA - ${{ github.sha }} + uses: actions/checkout@v4 + with: + repository: open-telemetry/opentelemetry-python + path: opentelemetry-python-core + + - name: Set up Python 3.8 + uses: actions/setup-python@v5 + with: + python-version: "3.8" + architecture: "x64" + + - name: Install tox + run: pip install tox + + - name: Run tests + run: tox -e py38-test-sdk-extension-aws-0 -- -ra + + py38-test-sdk-extension-aws-1: + name: sdk-extension-aws-1 + runs-on: ubuntu-latest + steps: + - name: Checkout contrib repo @ SHA - ${{ env.CONTRIB_REPO_SHA }} + uses: actions/checkout@v4 + with: + repository: open-telemetry/opentelemetry-python-contrib + ref: ${{ env.CONTRIB_REPO_SHA }} + + - name: Checkout core repo @ SHA - ${{ github.sha }} + uses: actions/checkout@v4 + with: + repository: open-telemetry/opentelemetry-python + path: opentelemetry-python-core + + - name: Set up Python 3.8 + uses: actions/setup-python@v5 + with: + python-version: "3.8" + architecture: "x64" + + - name: Install tox + run: pip install tox + + - name: Run tests + run: tox -e py38-test-sdk-extension-aws-1 -- -ra py38-test-distro: name: distro @@ -1584,6 +1668,34 @@ jobs: - name: Run tests run: tox -e py38-test-instrumentation-sqlalchemy-1 -- -ra + py38-test-instrumentation-sqlalchemy-2: + name: instrumentation-sqlalchemy-2 + runs-on: ubuntu-latest + steps: + - name: Checkout contrib repo @ SHA - ${{ env.CONTRIB_REPO_SHA }} + uses: actions/checkout@v4 + with: + repository: open-telemetry/opentelemetry-python-contrib + ref: ${{ env.CONTRIB_REPO_SHA }} + + - name: Checkout core repo @ SHA - ${{ github.sha }} + uses: actions/checkout@v4 + with: + repository: open-telemetry/opentelemetry-python + path: opentelemetry-python-core + + - name: Set up Python 3.8 + uses: actions/setup-python@v5 + with: + python-version: "3.8" + architecture: "x64" + + - name: Install tox + run: pip install tox + + - name: Run tests + run: tox -e py38-test-instrumentation-sqlalchemy-2 -- -ra + py38-test-instrumentation-redis: name: instrumentation-redis runs-on: ubuntu-latest @@ -1864,8 +1976,36 @@ jobs: - name: Run tests run: tox -e py38-test-util-http -- -ra - py38-test-propagator-aws-xray: - name: propagator-aws-xray + py38-test-propagator-aws-xray-0: + name: propagator-aws-xray-0 + runs-on: ubuntu-latest + steps: + - name: Checkout contrib repo @ SHA - ${{ env.CONTRIB_REPO_SHA }} + uses: actions/checkout@v4 + with: + repository: open-telemetry/opentelemetry-python-contrib + ref: ${{ env.CONTRIB_REPO_SHA }} + + - name: Checkout core repo @ SHA - ${{ github.sha }} + uses: actions/checkout@v4 + with: + repository: open-telemetry/opentelemetry-python + path: opentelemetry-python-core + + - name: Set up Python 3.8 + uses: actions/setup-python@v5 + with: + python-version: "3.8" + architecture: "x64" + + - name: Install tox + run: pip install tox + + - name: Run tests + run: tox -e py38-test-propagator-aws-xray-0 -- -ra + + py38-test-propagator-aws-xray-1: + name: propagator-aws-xray-1 runs-on: ubuntu-latest steps: - name: Checkout contrib repo @ SHA - ${{ env.CONTRIB_REPO_SHA }} @@ -1890,7 +2030,7 @@ jobs: run: pip install tox - name: Run tests - run: tox -e py38-test-propagator-aws-xray -- -ra + run: tox -e py38-test-propagator-aws-xray-1 -- -ra py38-test-propagator-ot-trace: name: propagator-ot-trace From a2d77a03ffc7e014b5e69bf5d6fbce23976637ba Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 7 Nov 2024 17:05:43 -0800 Subject: [PATCH 3/6] Update .readthedocs.yml (#4257) --- .readthedocs.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.readthedocs.yml b/.readthedocs.yml index 2a3c920b45..6769c5d193 100644 --- a/.readthedocs.yml +++ b/.readthedocs.yml @@ -2,13 +2,13 @@ # See https://docs.readthedocs.io/en/stable/config-file/v2.html for details version: 2 -sphinx: - configuration: docs/conf.py - build: os: "ubuntu-22.04" tools: python: "3.8" + +sphinx: + configuration: docs/conf.py python: install: From c4fa8d70fccecef0f853d542b229de9e20a20cd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Fri, 8 Nov 2024 17:23:08 +0100 Subject: [PATCH 4/6] Fix: filter exemplar for observable instrument and export of exemplar without trace and span ids (#4251) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 Co-authored-by: Riccardo Magliocchetti Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> Co-authored-by: Aaron Abbott --- CHANGELOG.md | 3 + .../_internal/metrics_encoder/__init__.py | 33 +- .../tests/test_metrics_encoder.py | 167 ++++++++- .../metrics/_internal/exemplar/exemplar.py | 4 +- .../_internal/exemplar/exemplar_reservoir.py | 4 +- .../metrics/_internal/measurement_consumer.py | 28 +- .../integration_test/test_exemplars.py | 317 ++++++++++++++++++ .../metrics/test_measurement_consumer.py | 5 +- 8 files changed, 532 insertions(+), 29 deletions(-) create mode 100644 opentelemetry-sdk/tests/metrics/integration_test/test_exemplars.py diff --git a/CHANGELOG.md b/CHANGELOG.md index e7f7bc6900..3006046690 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/metrics_encoder/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/metrics_encoder/__init__.py index 9f2b27d5a5..0c2b153b3b 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/metrics_encoder/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/metrics_encoder/__init__.py @@ -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, @@ -34,6 +34,7 @@ ) from opentelemetry.sdk.metrics import ( Counter, + Exemplar, Histogram, ObservableCounter, ObservableGauge, @@ -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. @@ -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 diff --git a/exporter/opentelemetry-exporter-otlp-proto-common/tests/test_metrics_encoder.py b/exporter/opentelemetry-exporter-otlp-proto-common/tests/test_metrics_encoder.py index 44092bdc18..cdf1296485 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-common/tests/test_metrics_encoder.py +++ b/exporter/opentelemetry-exporter-otlp-proto-common/tests/test_metrics_encoder.py @@ -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 ( @@ -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, @@ -55,6 +56,9 @@ class TestOTLPMetricsEncoder(unittest.TestCase): + span_id = int("6e0c63257de34c92", 16) + trace_id = int("d4cda95b652f4a1592b449d5929fda1b", 16) + histogram = Metric( name="histogram", description="foo", @@ -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], @@ -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, ) @@ -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, ) @@ -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, ) @@ -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, ) @@ -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, ) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar.py index d3199c69ab..95582e1601 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar.py @@ -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 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py index 1dcbfe47da..c8fa7f1453 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py @@ -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( diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/measurement_consumer.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/measurement_consumer.py index 2acbe1734c..c651033051 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/measurement_consumer.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/measurement_consumer.py @@ -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( @@ -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() diff --git a/opentelemetry-sdk/tests/metrics/integration_test/test_exemplars.py b/opentelemetry-sdk/tests/metrics/integration_test/test_exemplars.py new file mode 100644 index 0000000000..c4dabe9209 --- /dev/null +++ b/opentelemetry-sdk/tests/metrics/integration_test/test_exemplars.py @@ -0,0 +1,317 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import os +from unittest import TestCase, mock + +from opentelemetry import trace as trace_api +from opentelemetry.sdk.metrics import Exemplar, MeterProvider +from opentelemetry.sdk.metrics.export import ( + AggregationTemporality, + InMemoryMetricReader, + Metric, + NumberDataPoint, + Sum, +) +from opentelemetry.trace import SpanContext, TraceFlags + + +class TestExemplars(TestCase): + TRACE_ID = int("d4cda95b652f4a1592b449d5929fda1b", 16) + SPAN_ID = int("6e0c63257de34c92", 16) + + @mock.patch.dict(os.environ, {"OTEL_METRICS_EXEMPLAR_FILTER": "always_on"}) + def test_always_on_exemplars(self): + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + ) + meter = meter_provider.get_meter("testmeter") + counter = meter.create_counter("testcounter") + counter.add(10, {"label": "value1"}) + data = reader.get_metrics_data() + metrics = data.resource_metrics[0].scope_metrics[0].metrics + self.assertEqual( + metrics, + [ + Metric( + name="testcounter", + description="", + unit="", + data=Sum( + data_points=[ + NumberDataPoint( + attributes={"label": "value1"}, + start_time_unix_nano=mock.ANY, + time_unix_nano=mock.ANY, + value=10, + exemplars=[ + Exemplar( + filtered_attributes={}, + value=10, + time_unix_nano=mock.ANY, + span_id=None, + trace_id=None, + ), + ], + ) + ], + aggregation_temporality=AggregationTemporality.CUMULATIVE, + is_monotonic=True, + ), + ) + ], + ) + + @mock.patch.dict( + os.environ, {"OTEL_METRICS_EXEMPLAR_FILTER": "trace_based"} + ) + def test_trace_based_exemplars(self): + span_context = SpanContext( + trace_id=self.TRACE_ID, + span_id=self.SPAN_ID, + is_remote=False, + trace_flags=TraceFlags(TraceFlags.SAMPLED), + trace_state={}, + ) + span = trace_api.NonRecordingSpan(span_context) + trace_api.set_span_in_context(span) + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + ) + + meter = meter_provider.get_meter("testmeter") + counter = meter.create_counter("testcounter") + with trace_api.use_span(span): + counter.add(10, {"label": "value1"}) + data = reader.get_metrics_data() + metrics = data.resource_metrics[0].scope_metrics[0].metrics + self.assertEqual( + metrics, + [ + Metric( + name="testcounter", + description="", + unit="", + data=Sum( + data_points=[ + NumberDataPoint( + attributes={"label": "value1"}, + start_time_unix_nano=mock.ANY, + time_unix_nano=mock.ANY, + value=10, + exemplars=[ + Exemplar( + filtered_attributes={}, + value=10, + time_unix_nano=mock.ANY, + span_id=self.SPAN_ID, + trace_id=self.TRACE_ID, + ), + ], + ) + ], + aggregation_temporality=AggregationTemporality.CUMULATIVE, + is_monotonic=True, + ), + ) + ], + ) + + def test_default_exemplar_filter_no_span(self): + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + ) + + meter = meter_provider.get_meter("testmeter") + counter = meter.create_counter("testcounter") + counter.add(10, {"label": "value1"}) + data = reader.get_metrics_data() + metrics = data.resource_metrics[0].scope_metrics[0].metrics + self.assertEqual( + metrics, + [ + Metric( + name="testcounter", + description="", + unit="", + data=Sum( + data_points=[ + NumberDataPoint( + attributes={"label": "value1"}, + start_time_unix_nano=mock.ANY, + time_unix_nano=mock.ANY, + value=10, + exemplars=[], + ) + ], + aggregation_temporality=AggregationTemporality.CUMULATIVE, + is_monotonic=True, + ), + ) + ], + ) + + def test_default_exemplar_filter(self): + span_context = SpanContext( + trace_id=self.TRACE_ID, + span_id=self.SPAN_ID, + is_remote=False, + trace_flags=TraceFlags(TraceFlags.SAMPLED), + trace_state={}, + ) + span = trace_api.NonRecordingSpan(span_context) + trace_api.set_span_in_context(span) + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + ) + + meter = meter_provider.get_meter("testmeter") + counter = meter.create_counter("testcounter") + with trace_api.use_span(span): + counter.add(10, {"label": "value1"}) + data = reader.get_metrics_data() + metrics = data.resource_metrics[0].scope_metrics[0].metrics + self.assertEqual( + metrics, + [ + Metric( + name="testcounter", + description="", + unit="", + data=Sum( + data_points=[ + NumberDataPoint( + attributes={"label": "value1"}, + start_time_unix_nano=mock.ANY, + time_unix_nano=mock.ANY, + value=10, + exemplars=[ + Exemplar( + filtered_attributes={}, + value=10, + time_unix_nano=mock.ANY, + span_id=self.SPAN_ID, + trace_id=self.TRACE_ID, + ), + ], + ) + ], + aggregation_temporality=AggregationTemporality.CUMULATIVE, + is_monotonic=True, + ), + ) + ], + ) + + def test_exemplar_trace_based_manual_context(self): + span_context = SpanContext( + trace_id=self.TRACE_ID, + span_id=self.SPAN_ID, + is_remote=False, + trace_flags=TraceFlags(TraceFlags.SAMPLED), + trace_state={}, + ) + span = trace_api.NonRecordingSpan(span_context) + ctx = trace_api.set_span_in_context(span) + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + ) + + meter = meter_provider.get_meter("testmeter") + counter = meter.create_counter("testcounter") + counter.add(10, {"label": "value1"}, context=ctx) + data = reader.get_metrics_data() + metrics = data.resource_metrics[0].scope_metrics[0].metrics + self.assertEqual( + metrics, + [ + Metric( + name="testcounter", + description="", + unit="", + data=Sum( + data_points=[ + NumberDataPoint( + attributes={"label": "value1"}, + start_time_unix_nano=mock.ANY, + time_unix_nano=mock.ANY, + value=10, + exemplars=[ + Exemplar( + filtered_attributes={}, + value=10, + time_unix_nano=mock.ANY, + span_id=self.SPAN_ID, + trace_id=self.TRACE_ID, + ), + ], + ) + ], + aggregation_temporality=AggregationTemporality.CUMULATIVE, + is_monotonic=True, + ), + ) + ], + ) + + @mock.patch.dict( + os.environ, {"OTEL_METRICS_EXEMPLAR_FILTER": "always_off"} + ) + def test_always_off_exemplars(self): + span_context = SpanContext( + trace_id=self.TRACE_ID, + span_id=self.SPAN_ID, + is_remote=False, + trace_flags=TraceFlags(TraceFlags.SAMPLED), + trace_state={}, + ) + span = trace_api.NonRecordingSpan(span_context) + trace_api.set_span_in_context(span) + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + ) + meter = meter_provider.get_meter("testmeter") + counter = meter.create_counter("testcounter") + with trace_api.use_span(span): + counter.add(10, {"label": "value1"}) + data = reader.get_metrics_data() + metrics = data.resource_metrics[0].scope_metrics[0].metrics + self.assertEqual( + metrics, + [ + Metric( + name="testcounter", + description="", + unit="", + data=Sum( + data_points=[ + NumberDataPoint( + attributes={"label": "value1"}, + start_time_unix_nano=mock.ANY, + time_unix_nano=mock.ANY, + value=10, + exemplars=[], + ) + ], + aggregation_temporality=AggregationTemporality.CUMULATIVE, + is_monotonic=True, + ), + ) + ], + ) diff --git a/opentelemetry-sdk/tests/metrics/test_measurement_consumer.py b/opentelemetry-sdk/tests/metrics/test_measurement_consumer.py index fe58ec4aca..22abfbd3cf 100644 --- a/opentelemetry-sdk/tests/metrics/test_measurement_consumer.py +++ b/opentelemetry-sdk/tests/metrics/test_measurement_consumer.py @@ -100,7 +100,7 @@ def test_collect_calls_async_instruments(self, MockMetricReaderStorage): MockMetricReaderStorage.return_value = reader_storage_mock consumer = SynchronousMeasurementConsumer( SdkConfiguration( - exemplar_filter=Mock(), + exemplar_filter=Mock(should_sample=Mock(return_value=False)), resource=Mock(), metric_readers=[reader_mock], views=Mock(), @@ -121,6 +121,9 @@ def test_collect_calls_async_instruments(self, MockMetricReaderStorage): self.assertEqual( len(reader_storage_mock.consume_measurement.mock_calls), 5 ) + # assert consume_measurement was called with at least 2 arguments the second + # matching the mocked exemplar filter + self.assertFalse(reader_storage_mock.consume_measurement.call_args[1]) def test_collect_timeout(self, MockMetricReaderStorage): reader_mock = Mock() From f194bc3bf02a78d1ea0f5de8e4b7c0781d29f613 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Fri, 8 Nov 2024 17:50:44 +0100 Subject: [PATCH 5/6] opentelemetry-sdk: avoid recursion error with sdk disabled (#4259) * opentelemetry-sdk: avoid recursion error with sdk disabled If the sdk is disable and our handler is added to the root logger we will recurse in order to log that SDK is disabled. Use the warnings facilities to print the message instead. * Add changelog --- CHANGELOG.md | 2 ++ .../sdk/_logs/_internal/__init__.py | 2 +- opentelemetry-sdk/tests/logs/test_handler.py | 24 ++++++++++++++++--- opentelemetry-sdk/tests/logs/test_logs.py | 7 +++++- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3006046690..092a13ce01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix metrics export with exemplar and no context and filtering observable instruments ([#4251](https://github.com/open-telemetry/opentelemetry-python/pull/4251)) +- Fix recursion error with sdk disabled and handler added to root logger + ([#4259](https://github.com/open-telemetry/opentelemetry-python/pull/4259)) ## Version 1.28.0/0.49b0 (2024-11-05) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index ed4ec38801..2d52b1bc74 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -692,7 +692,7 @@ def get_logger( attributes: Optional[Attributes] = None, ) -> Logger: if self._disabled: - _logger.warning("SDK is disabled.") + warnings.warn("SDK is disabled.") return NoOpLogger( name, version=version, diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index e8f70d6f5c..f6daa1b22c 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -11,9 +11,12 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + import logging +import os import unittest -from unittest.mock import Mock +import warnings +from unittest.mock import Mock, patch from opentelemetry._logs import NoOpLoggerProvider, SeverityNumber from opentelemetry._logs import get_logger as APIGetLogger @@ -280,12 +283,27 @@ def test_log_body_is_always_string_with_formatter(self): log_record = processor.get_log_record(0) self.assertIsInstance(log_record.body, str) + @patch.dict(os.environ, {"OTEL_SDK_DISABLED": "true"}) + def test_handler_root_logger_with_disabled_sdk_does_not_go_into_recursion_error( + self, + ): + processor, logger = set_up_test_logging( + logging.NOTSET, root_logger=True + ) + with warnings.catch_warnings(record=True) as cw: + logger.warning("hello") + + self.assertEqual(len(cw), 1) + self.assertEqual("SDK is disabled.", str(cw[0].message)) + + self.assertEqual(processor.emit_count(), 0) + -def set_up_test_logging(level, formatter=None): +def set_up_test_logging(level, formatter=None, root_logger=False): logger_provider = LoggerProvider() processor = FakeProcessor() logger_provider.add_log_record_processor(processor) - logger = logging.getLogger("foo") + logger = logging.getLogger(None if root_logger else "foo") handler = LoggingHandler(level=level, logger_provider=logger_provider) if formatter: handler.setFormatter(formatter) diff --git a/opentelemetry-sdk/tests/logs/test_logs.py b/opentelemetry-sdk/tests/logs/test_logs.py index 4cb2a46c00..0590669653 100644 --- a/opentelemetry-sdk/tests/logs/test_logs.py +++ b/opentelemetry-sdk/tests/logs/test_logs.py @@ -15,6 +15,7 @@ # pylint: disable=protected-access import unittest +import warnings from unittest.mock import Mock, patch from opentelemetry.sdk._logs import LoggerProvider @@ -69,8 +70,12 @@ def test_get_logger(self): @patch.dict("os.environ", {OTEL_SDK_DISABLED: "true"}) def test_get_logger_with_sdk_disabled(self): - logger = LoggerProvider().get_logger(Mock()) + with warnings.catch_warnings(record=True) as cw: + logger = LoggerProvider().get_logger(Mock()) + self.assertIsInstance(logger, NoOpLogger) + self.assertEqual(len(cw), 1) + self.assertEqual("SDK is disabled.", str(cw[0].message)) @patch.object(Resource, "create") def test_logger_provider_init(self, resource_patch): From ca774ee86e9dd7cde48c20b7999831f3cb4b0391 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 8 Nov 2024 18:32:57 -0800 Subject: [PATCH 6/6] toml (#4265) --- .github/workflows/prepare-patch-release.yml | 3 +++ .github/workflows/prepare-release-branch.yml | 3 +++ .github/workflows/release.yml | 3 +++ 3 files changed, 9 insertions(+) diff --git a/.github/workflows/prepare-patch-release.yml b/.github/workflows/prepare-patch-release.yml index 90aa911d2f..1f68f37ce2 100644 --- a/.github/workflows/prepare-patch-release.yml +++ b/.github/workflows/prepare-patch-release.yml @@ -8,6 +8,9 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Install toml + run: pip install toml + - run: | if [[ ! $GITHUB_REF_NAME =~ ^release/v[0-9]+\.[0-9]+\.x-0\.[0-9]+bx$ ]]; then echo this workflow should only be run against long-term release branches diff --git a/.github/workflows/prepare-release-branch.yml b/.github/workflows/prepare-release-branch.yml index 3a19de88df..18bad26bfb 100644 --- a/.github/workflows/prepare-release-branch.yml +++ b/.github/workflows/prepare-release-branch.yml @@ -12,6 +12,9 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Install toml + run: pip install toml + - name: Verify prerequisites env: PRERELEASE_VERSION: ${{ github.event.inputs.prerelease_version }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b45ff25e71..447b5f6dfc 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -14,6 +14,9 @@ jobs: - uses: actions/checkout@v4 + - name: Install toml + run: pip install toml + - name: Set environment variables run: | stable_version=$(./scripts/eachdist.py version --mode stable)