From 93e53993e73834772c90a85d3963eb11021886cc Mon Sep 17 00:00:00 2001 From: alrex Date: Wed, 29 Sep 2021 11:30:47 -0700 Subject: [PATCH 01/13] add assertTraceResponseHeaderMatchesSpan method (#2159) * add assertTraceResponseHeaderMatchesSpan method * fix lint Co-authored-by: Diego Hurtado --- .../util/src/opentelemetry/test/wsgitestutil.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/util/src/opentelemetry/test/wsgitestutil.py b/tests/util/src/opentelemetry/test/wsgitestutil.py index 349db8f6944..aa2379a2b72 100644 --- a/tests/util/src/opentelemetry/test/wsgitestutil.py +++ b/tests/util/src/opentelemetry/test/wsgitestutil.py @@ -15,6 +15,7 @@ import io import wsgiref.util as wsgiref_util +from opentelemetry import trace from opentelemetry.test.spantestutil import SpanTestBase @@ -37,3 +38,19 @@ def start_response(self, status, response_headers, exc_info=None): self.response_headers = response_headers self.exc_info = exc_info return self.write + + def assertTraceResponseHeaderMatchesSpan( + self, headers, span + ): # pylint: disable=invalid-name + self.assertIn("traceresponse", headers) + self.assertEqual( + headers["access-control-expose-headers"], + "traceresponse", + ) + + trace_id = trace.format_trace_id(span.get_span_context().trace_id) + span_id = trace.format_span_id(span.get_span_context().span_id) + self.assertEqual( + f"00-{trace_id}-{span_id}-01", + headers["traceresponse"], + ) From b20a6c5095717981ffba11d8244aa10a8f22890a Mon Sep 17 00:00:00 2001 From: alrex Date: Wed, 29 Sep 2021 12:39:03 -0700 Subject: [PATCH 02/13] updating bootstrap_gen (#2158) Co-authored-by: Diego Hurtado --- .../src/opentelemetry/instrumentation/bootstrap_gen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py index e400a3417b4..06bad91a220 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py @@ -53,7 +53,7 @@ "instrumentation": "opentelemetry-instrumentation-elasticsearch==0.24b0", }, "falcon": { - "library": "falcon ~= 2.0", + "library": "falcon >= 2.0.0, < 4.0.0", "instrumentation": "opentelemetry-instrumentation-falcon==0.24b0", }, "fastapi": { From 18b5cb046bebab0c70841c662268bb7980c2fd5e Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 29 Sep 2021 23:28:56 +0200 Subject: [PATCH 03/13] Load environment variables as options for opentelemetry-instrument (#1969) --- CHANGELOG.md | 3 +- CONTRIBUTING.md | 10 +++ opentelemetry-api/setup.cfg | 2 + .../__init__.py => environment_variables.py} | 5 -- .../src/opentelemetry/distro/__init__.py | 2 +- opentelemetry-instrumentation/setup.cfg | 2 + .../auto_instrumentation/__init__.py | 80 ++++++++++--------- .../auto_instrumentation/sitecustomize.py | 6 +- .../instrumentation/environment_variables.py | 18 +++++ .../tests/test_run.py | 3 +- opentelemetry-sdk/setup.cfg | 2 + .../__init__.py => environment_variables.py} | 0 12 files changed, 83 insertions(+), 50 deletions(-) rename opentelemetry-api/src/opentelemetry/{environment_variables/__init__.py => environment_variables.py} (88%) create mode 100644 opentelemetry-instrumentation/src/opentelemetry/instrumentation/environment_variables.py rename opentelemetry-sdk/src/opentelemetry/sdk/{environment_variables/__init__.py => environment_variables.py} (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d9a92b2e44..835601d58ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.5.0-0.24b0...HEAD) - +- Automatically load OTEL environment variables as options for `opentelemetry-instrument` + ([#1969](https://github.com/open-telemetry/opentelemetry-python/pull/1969)) - `opentelemetry-semantic-conventions` Update to semantic conventions v1.6.1 ([#2077](https://github.com/open-telemetry/opentelemetry-python/pull/2077)) - Do not count invalid attributes for dropped diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7084a8fbca1..ac76ccc70d0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -214,6 +214,16 @@ rather than conform to specific API names or argument patterns in the spec. For a deeper discussion, see: https://github.com/open-telemetry/opentelemetry-specification/issues/165 +### Environment Variables + +If you are adding a component that introduces new OpenTelemetry environment variables, put them all in a module, +as it is done in `opentelemetry.environment_variables` or in `opentelemetry.sdk.environment_variables`. + +Keep in mind that any new environment variable must be declared in all caps and must start with `OTEL_PYTHON_`. + +Register this module with the `opentelemetry_environment_variables` entry point to make your environment variables +automatically load as options for the `opentelemetry-instrument` command. + ## Style Guide * docstrings should adhere to the [Google Python Style diff --git a/opentelemetry-api/setup.cfg b/opentelemetry-api/setup.cfg index 99d52838fb1..91a38ac9278 100644 --- a/opentelemetry-api/setup.cfg +++ b/opentelemetry-api/setup.cfg @@ -56,6 +56,8 @@ opentelemetry_tracer_provider = opentelemetry_propagator = tracecontext = opentelemetry.trace.propagation.tracecontext:TraceContextTextMapPropagator baggage = opentelemetry.baggage.propagation:W3CBaggagePropagator +opentelemetry_environment_variables = + api = opentelemetry.environment_variables [options.extras_require] test = diff --git a/opentelemetry-api/src/opentelemetry/environment_variables/__init__.py b/opentelemetry-api/src/opentelemetry/environment_variables.py similarity index 88% rename from opentelemetry-api/src/opentelemetry/environment_variables/__init__.py rename to opentelemetry-api/src/opentelemetry/environment_variables.py index 6077115b013..83ec67149dd 100644 --- a/opentelemetry-api/src/opentelemetry/environment_variables/__init__.py +++ b/opentelemetry-api/src/opentelemetry/environment_variables.py @@ -22,11 +22,6 @@ .. envvar:: OTEL_PYTHON_CONTEXT """ -OTEL_PYTHON_DISABLED_INSTRUMENTATIONS = "OTEL_PYTHON_DISABLED_INSTRUMENTATIONS" -""" -.. envvar:: OTEL_PYTHON_DISABLED_INSTRUMENTATIONS -""" - OTEL_PYTHON_ID_GENERATOR = "OTEL_PYTHON_ID_GENERATOR" """ .. envvar:: OTEL_PYTHON_ID_GENERATOR diff --git a/opentelemetry-distro/src/opentelemetry/distro/__init__.py b/opentelemetry-distro/src/opentelemetry/distro/__init__.py index e70cb67335d..97e3e2fcc94 100644 --- a/opentelemetry-distro/src/opentelemetry/distro/__init__.py +++ b/opentelemetry-distro/src/opentelemetry/distro/__init__.py @@ -11,7 +11,7 @@ # 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 opentelemetry.environment_variables import OTEL_TRACES_EXPORTER diff --git a/opentelemetry-instrumentation/setup.cfg b/opentelemetry-instrumentation/setup.cfg index 6ecd9d75e69..042f0edb198 100644 --- a/opentelemetry-instrumentation/setup.cfg +++ b/opentelemetry-instrumentation/setup.cfg @@ -51,6 +51,8 @@ where = src console_scripts = opentelemetry-instrument = opentelemetry.instrumentation.auto_instrumentation:run opentelemetry-bootstrap = opentelemetry.instrumentation.bootstrap:run +opentelemetry_environment_variables = + instrumentation = opentelemetry.instrumentation.environment_variables [options.extras_require] test = diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py index 9f076b340e5..29b09a0c347 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/__init__.py @@ -14,72 +14,74 @@ # See the License for the specific language governing permissions and # limitations under the License. -import argparse +from argparse import REMAINDER, ArgumentParser from logging import getLogger from os import environ, execl, getcwd from os.path import abspath, dirname, pathsep +from re import sub from shutil import which -from opentelemetry.environment_variables import ( - OTEL_PYTHON_ID_GENERATOR, - OTEL_TRACES_EXPORTER, -) +from pkg_resources import iter_entry_points -logger = getLogger(__file__) +_logger = getLogger(__file__) -def parse_args(): - parser = argparse.ArgumentParser( +def run() -> None: + + parser = ArgumentParser( description=""" opentelemetry-instrument automatically instruments a Python program and its dependencies and then runs the program. - """ + """, + epilog=""" + Optional arguments (except for --help) for opentelemetry-instrument + directly correspond with OpenTelemetry environment variables. The + corresponding optional argument is formed by removing the OTEL_ or + OTEL_PYTHON_ prefix from the environment variable and lower casing the + rest. For example, the optional argument --attribute_value_length_limit + corresponds with the environment variable + OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT. + + These optional arguments will override the current value of the + corresponding environment variable during the execution of the command. + """, ) - parser.add_argument( - "--trace-exporter", - required=False, - help=""" - Uses the specified exporter to export spans. - Accepts multiple exporters as comma separated values. + argument_otel_environment_variable = {} - Examples: + for entry_point in iter_entry_points( + "opentelemetry_environment_variables" + ): + environment_variable_module = entry_point.load() - --trace-exporter=jaeger - """, - ) + for attribute in dir(environment_variable_module): - parser.add_argument( - "--id-generator", - required=False, - help=""" - The IDs Generator to be used with the Tracer Provider. + if attribute.startswith("OTEL_"): - Examples: + argument = sub(r"OTEL_(PYTHON_)?", "", attribute).lower() - --id-generator=random - """, - ) + parser.add_argument( + f"--{argument}", + required=False, + ) + argument_otel_environment_variable[argument] = attribute parser.add_argument("command", help="Your Python application.") parser.add_argument( "command_args", help="Arguments for your application.", - nargs=argparse.REMAINDER, + nargs=REMAINDER, ) - return parser.parse_args() - -def load_config_from_cli_args(args): - if args.trace_exporter: - environ[OTEL_TRACES_EXPORTER] = args.trace_exporter - if args.id_generator: - environ[OTEL_PYTHON_ID_GENERATOR] = args.id_generator + args = parser.parse_args() + for argument, otel_environment_variable in ( + argument_otel_environment_variable + ).items(): + value = getattr(args, argument) + if value is not None: -def run() -> None: - args = parse_args() - load_config_from_cli_args(args) + environ[otel_environment_variable] = value python_path = environ.get("PYTHONPATH") diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py index a4a5dc8d5a0..f7a6412ff6e 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py @@ -20,13 +20,13 @@ from pkg_resources import iter_entry_points -from opentelemetry.environment_variables import ( - OTEL_PYTHON_DISABLED_INSTRUMENTATIONS, -) from opentelemetry.instrumentation.dependencies import ( get_dist_dependency_conflicts, ) from opentelemetry.instrumentation.distro import BaseDistro, DefaultDistro +from opentelemetry.instrumentation.environment_variables import ( + OTEL_PYTHON_DISABLED_INSTRUMENTATIONS, +) logger = getLogger(__file__) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/environment_variables.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/environment_variables.py new file mode 100644 index 00000000000..ad28f068590 --- /dev/null +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/environment_variables.py @@ -0,0 +1,18 @@ +# 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. + +OTEL_PYTHON_DISABLED_INSTRUMENTATIONS = "OTEL_PYTHON_DISABLED_INSTRUMENTATIONS" +""" +.. envvar:: OTEL_PYTHON_DISABLED_INSTRUMENTATIONS +""" diff --git a/opentelemetry-instrumentation/tests/test_run.py b/opentelemetry-instrumentation/tests/test_run.py index 01bd86ed32f..9fd3a21711e 100644 --- a/opentelemetry-instrumentation/tests/test_run.py +++ b/opentelemetry-instrumentation/tests/test_run.py @@ -111,7 +111,8 @@ def test_exporter(self, _): # pylint: disable=no-self-use self.assertIsNone(environ.get(OTEL_TRACES_EXPORTER)) with patch( - "sys.argv", ["instrument", "--trace-exporter", "jaeger", "1", "2"] + "sys.argv", + ["instrument", "--traces_exporter", "jaeger", "1", "2"], ): auto_instrumentation.run() self.assertEqual(environ.get(OTEL_TRACES_EXPORTER), "jaeger") diff --git a/opentelemetry-sdk/setup.cfg b/opentelemetry-sdk/setup.cfg index 0c699262c64..56390868ed1 100644 --- a/opentelemetry-sdk/setup.cfg +++ b/opentelemetry-sdk/setup.cfg @@ -56,6 +56,8 @@ opentelemetry_traces_exporter = console = opentelemetry.sdk.trace.export:ConsoleSpanExporter opentelemetry_id_generator = random = opentelemetry.sdk.trace.id_generator:RandomIdGenerator +opentelemetry_environment_variables = + sdk = opentelemetry.sdk.environment_variables [options.extras_require] test = diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py similarity index 100% rename from opentelemetry-sdk/src/opentelemetry/sdk/environment_variables/__init__.py rename to opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py From 9554bbc4c8e60c7bba16a7f07b2f9fadc9e75d36 Mon Sep 17 00:00:00 2001 From: Patrice Chalin Date: Mon, 4 Oct 2021 15:51:55 -0400 Subject: [PATCH 04/13] Delete docs-update.yml (#2171) --- .github/workflows/docs-update.yml | 35 ------------------------------- 1 file changed, 35 deletions(-) delete mode 100644 .github/workflows/docs-update.yml diff --git a/.github/workflows/docs-update.yml b/.github/workflows/docs-update.yml deleted file mode 100644 index 78927177766..00000000000 --- a/.github/workflows/docs-update.yml +++ /dev/null @@ -1,35 +0,0 @@ -name: Update OpenTelemetry Website Docs - -on: - # triggers only on a manual dispatch - workflow_dispatch: - -jobs: - update-docs: - runs-on: ubuntu-latest - steps: - - name: checkout - uses: actions/checkout@v2 - - name: make-pr - env: - API_TOKEN_GITHUB: ${{secrets.DOC_UPDATE_TOKEN}} - # Destination repo should always be 'open-telemetry/opentelemetry.io' - DESTINATION_REPO: open-telemetry/opentelemetry.io - # Destination path should be the absolute path to your language's friendly name in the docs tree (i.e, 'content/en/docs/python') - DESTINATION_PATH: content/en/docs/python - # Source path should be 'website_docs', all files and folders are copied from here to dest - SOURCE_PATH: website_docs - run: | - TARGET_DIR=$(mktemp -d) - export GITHUB_TOKEN=$API_TOKEN_GITHUB - git config --global user.name austinlparker - git config --global user.email austin@lightstep.com - git clone "https://$API_TOKEN_GITHUB@github.com/$DESTINATION_REPO.git" "$TARGET_DIR" - rsync -av --delete "$SOURCE_PATH/" "$TARGET_DIR/$DESTINATION_PATH/" - cd "$TARGET_DIR" - git checkout -b docs-$GITHUB_REPOSITORY-$GITHUB_SHA - git add . - git commit -m "Docs update from $GITHUB_REPOSITORY" - git push -u origin HEAD:docs-$GITHUB_REPOSITORY-$GITHUB_SHA - gh pr create -t "Docs Update from $GITHUB_REPOSITORY" -b "This is an automated pull request." -B main -H docs-$GITHUB_REPOSITORY-$GITHUB_SHA - echo "done" From c9b18c66229da61065846ba9ace25a5f5c2106b8 Mon Sep 17 00:00:00 2001 From: Nikolay Sokolik <81902191+oxeye-nikolay@users.noreply.github.com> Date: Tue, 5 Oct 2021 14:55:44 +0300 Subject: [PATCH 05/13] Updated bootstrap_gen.py for pika instrumentation in the contrib (#2160) Co-authored-by: Owais Lone Co-authored-by: Diego Hurtado --- .../src/opentelemetry/instrumentation/bootstrap_gen.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py index 06bad91a220..851c03d5f3f 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py @@ -80,6 +80,10 @@ "library": "mysql-connector-python ~= 8.0", "instrumentation": "opentelemetry-instrumentation-mysql==0.24b0", }, + "pika": { + "library": "pika >= 1.1.0", + "instrumentation": "opentelemetry-instrumentation-pika==0.24b0", + }, "psycopg2": { "library": "psycopg2 >= 2.7.3.1", "instrumentation": "opentelemetry-instrumentation-psycopg2==0.24b0", From a0d1267b3aec027b72c39296ea350fa5210332fc Mon Sep 17 00:00:00 2001 From: andrew-matteson Date: Wed, 6 Oct 2021 11:19:10 -0400 Subject: [PATCH 06/13] Update bootstrap_gen.py for jinja2 versions (#2183) --- .../src/opentelemetry/instrumentation/bootstrap_gen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py index 851c03d5f3f..efd37e52661 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py @@ -73,7 +73,7 @@ "instrumentation": "opentelemetry-instrumentation-httpx==0.24b0", }, "jinja2": { - "library": "jinja2~=2.7", + "library": "jinja2 >= 2.7, < 4.0", "instrumentation": "opentelemetry-instrumentation-jinja2==0.24b0", }, "mysql-connector-python": { From 65528f7534f1f5f2e8adc7520b6e696a84569c7d Mon Sep 17 00:00:00 2001 From: alrex Date: Wed, 6 Oct 2021 14:13:03 -0700 Subject: [PATCH 07/13] update OTLP/HTTP example & test port to 4318 (#2016) * update OTLP/HTTP port to 4318 * map 4318->55681 in the collector for now * update development status Co-authored-by: Diego Hurtado --- exporter/opentelemetry-exporter-otlp-proto-http/setup.cfg | 2 +- .../exporter/otlp/proto/http/trace_exporter/__init__.py | 2 +- tests/opentelemetry-docker-tests/tests/docker-compose.yml | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/setup.cfg b/exporter/opentelemetry-exporter-otlp-proto-http/setup.cfg index 38b82f42656..9ea3711965e 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/setup.cfg +++ b/exporter/opentelemetry-exporter-otlp-proto-http/setup.cfg @@ -23,7 +23,7 @@ url = https://github.com/open-telemetry/opentelemetry-python/tree/main/exporter/ platforms = any license = Apache-2.0 classifiers = - Development Status :: 4 - Beta + Development Status :: 5 - Production/Stable Intended Audience :: Developers License :: OSI Approved :: Apache Software License Programming Language :: Python diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py index 211d037999c..0bcb0a7368c 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py @@ -47,7 +47,7 @@ DEFAULT_COMPRESSION = Compression.NoCompression -DEFAULT_ENDPOINT = "http://localhost:55681/v1/traces" +DEFAULT_ENDPOINT = "http://localhost:4318/v1/traces" DEFAULT_TIMEOUT = 10 # in seconds diff --git a/tests/opentelemetry-docker-tests/tests/docker-compose.yml b/tests/opentelemetry-docker-tests/tests/docker-compose.yml index b1cd2d6e1fe..3f74827156a 100644 --- a/tests/opentelemetry-docker-tests/tests/docker-compose.yml +++ b/tests/opentelemetry-docker-tests/tests/docker-compose.yml @@ -8,7 +8,7 @@ services: - "8888:8888" - "55678:55678" otcollector: - image: otel/opentelemetry-collector:0.22.0 + image: otel/opentelemetry-collector:0.31.0 ports: - "4317:4317" - - "55681:55681" + - "4318:55681" From 2a94c3d556f456160c4734be4bbd11fb34693d7c Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 12 Oct 2021 09:38:36 -0700 Subject: [PATCH 08/13] Update RELEASING.md (#2177) --- RELEASING.md | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/RELEASING.md b/RELEASING.md index b57632048f9..cc02d74b597 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -73,16 +73,6 @@ run eachdist once again: ./scripts/eachdist.py update_versions --versions stable,prerelease ``` -## Update website docs - -If the docs for the Opentelemetry [website](https://opentelemetry.io/docs/python/) was updated in this release in this [folder](https://github.com/open-telemetry/opentelemetry-python/tree/main/website_docs), submit a [PR](https://github.com/open-telemetry/opentelemetry.io/tree/main/content/en/docs/python) to update the docs on the website accordingly. To check if the new version requires updating, run the following script and compare the diff: - -```bash -./scripts/generate_website_docs.sh -``` - -If the diff includes significant changes, create a pull request to commit the changes and once the changes are merged, click the "Run workflow" button for the Update [OpenTelemetry Website Docs](https://github.com/open-telemetry/opentelemetry-python/actions/workflows/docs-update.yml) GitHub Action. - ## Hotfix procedure A `hotfix` is defined as a small change developed to correct a bug that should be released as quickly as possible. Due to the nature of hotfixes, they usually will only affect one or a few packages. Therefore, it usually is not necessary to go through the entire release process outlined above for hotfixes. Follow the below steps how to release a hotfix: From 0770fcd72f3dce435904e17e99a3795af5d64a08 Mon Sep 17 00:00:00 2001 From: Owais Lone Date: Tue, 12 Oct 2021 23:58:27 +0530 Subject: [PATCH 09/13] Update contrib SHA for Github Actions workflow (#2191) --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 795a288f2e8..14b8db0a5a5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,7 +10,7 @@ env: # Otherwise, set variable to the commit of your branch on # opentelemetry-python-contrib which is compatible with these Core repo # changes. - CONTRIB_REPO_SHA: d2984f5242ed2250ad1c11b6164e2e8e11e2a804 + CONTRIB_REPO_SHA: 36275f3cbf00c2021caa1d922bd98215c50b9af3 jobs: build: From 78672025f7fb49737356e5c50f7cb33992a3da55 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Tue, 12 Oct 2021 16:41:35 -0400 Subject: [PATCH 10/13] Fix race in `set_tracer_provider()` (#2182) * Fix race in set_tracer_provider * refactor _reset_globals to a test util * get rid of "Mixin" name and simplify code a bit * add some comments to concurrency_test.py * actually respect log option Co-authored-by: Diego Hurtado Co-authored-by: Leighton Chen Co-authored-by: Owais Lone --- CHANGELOG.md | 2 + .../tests/test_jaeger_exporter_thrift.py | 11 +-- .../tests/test_otcollector_trace_exporter.py | 11 +-- .../src/opentelemetry/trace/__init__.py | 40 +++++---- .../src/opentelemetry/util/_once.py | 47 ++++++++++ opentelemetry-api/tests/trace/test_globals.py | 65 +++++++++++--- opentelemetry-api/tests/trace/test_proxy.py | 10 +-- opentelemetry-api/tests/util/test_once.py | 48 ++++++++++ .../opentelemetry/test/concurrency_test.py | 90 +++++++++++++++++++ .../src/opentelemetry/test/globals_test.py | 41 +++++++++ .../util/src/opentelemetry/test/test_base.py | 7 +- tox.ini | 2 +- 12 files changed, 313 insertions(+), 61 deletions(-) create mode 100644 opentelemetry-api/src/opentelemetry/util/_once.py create mode 100644 opentelemetry-api/tests/util/test_once.py create mode 100644 tests/util/src/opentelemetry/test/concurrency_test.py create mode 100644 tests/util/src/opentelemetry/test/globals_test.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 835601d58ce..2523b5834d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.5.0-0.24b0...HEAD) +- Fix race in `set_tracer_provider()` + ([#2182](https://github.com/open-telemetry/opentelemetry-python/pull/2182)) - Automatically load OTEL environment variables as options for `opentelemetry-instrument` ([#1969](https://github.com/open-telemetry/opentelemetry-python/pull/1969)) - `opentelemetry-semantic-conventions` Update to semantic conventions v1.6.1 diff --git a/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py b/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py index c72cd579ff7..8a30527eebc 100644 --- a/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py +++ b/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py @@ -15,7 +15,6 @@ import unittest from unittest import mock -from unittest.mock import patch # pylint:disable=no-name-in-module # pylint:disable=import-error @@ -38,6 +37,7 @@ from opentelemetry.sdk.resources import SERVICE_NAME from opentelemetry.sdk.trace import Resource, TracerProvider from opentelemetry.sdk.util.instrumentation import InstrumentationInfo +from opentelemetry.test.globals_test import TraceGlobalsTest from opentelemetry.test.spantestutil import ( get_span_with_dropped_attributes_events_links, ) @@ -53,7 +53,7 @@ def _translate_spans_with_dropped_attributes(): return translate._translate(ThriftTranslator(max_tag_value_length=5)) -class TestJaegerExporter(unittest.TestCase): +class TestJaegerExporter(TraceGlobalsTest, unittest.TestCase): def setUp(self): # create and save span to be used in tests self.context = trace_api.SpanContext( @@ -73,7 +73,6 @@ def setUp(self): self._test_span.end(end_time=3) # pylint: disable=protected-access - @patch("opentelemetry.exporter.jaeger.thrift.trace._TRACER_PROVIDER", None) def test_constructor_default(self): # pylint: disable=protected-access """Test the default values assigned by constructor.""" @@ -98,7 +97,6 @@ def test_constructor_default(self): self.assertTrue(exporter._agent_client is not None) self.assertIsNone(exporter._max_tag_value_length) - @patch("opentelemetry.exporter.jaeger.thrift.trace._TRACER_PROVIDER", None) def test_constructor_explicit(self): # pylint: disable=protected-access """Test the constructor passing all the options.""" @@ -143,7 +141,6 @@ def test_constructor_explicit(self): self.assertTrue(exporter._collector_http_client.auth is None) self.assertEqual(exporter._max_tag_value_length, 42) - @patch("opentelemetry.exporter.jaeger.thrift.trace._TRACER_PROVIDER", None) def test_constructor_by_environment_variables(self): # pylint: disable=protected-access """Test the constructor using Environment Variables.""" @@ -198,7 +195,6 @@ def test_constructor_by_environment_variables(self): self.assertTrue(exporter._collector_http_client.auth is None) environ_patcher.stop() - @patch("opentelemetry.exporter.jaeger.thrift.trace._TRACER_PROVIDER", None) def test_constructor_with_no_traceprovider_resource(self): """Test the constructor when there is no resource attached to trace_provider""" @@ -480,7 +476,6 @@ def test_translate_to_jaeger(self): self.assertEqual(spans, expected_spans) - @patch("opentelemetry.exporter.jaeger.thrift.trace._TRACER_PROVIDER", None) def test_export(self): """Test that agent and/or collector are invoked""" @@ -511,9 +506,7 @@ def test_export(self): exporter.export((self._test_span,)) self.assertEqual(agent_client_mock.emit.call_count, 1) self.assertEqual(collector_mock.submit.call_count, 1) - # trace_api._TRACER_PROVIDER = None - @patch("opentelemetry.exporter.jaeger.thrift.trace._TRACER_PROVIDER", None) def test_export_span_service_name(self): trace_api.set_tracer_provider( TracerProvider( diff --git a/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_trace_exporter.py b/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_trace_exporter.py index 43d9bcd430b..cd4dcb1a08c 100644 --- a/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_trace_exporter.py +++ b/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_trace_exporter.py @@ -29,15 +29,12 @@ from opentelemetry.sdk.resources import SERVICE_NAME, Resource from opentelemetry.sdk.trace import TracerProvider from opentelemetry.sdk.trace.export import SpanExportResult +from opentelemetry.test.globals_test import TraceGlobalsTest from opentelemetry.trace import TraceFlags # pylint: disable=no-member -class TestCollectorSpanExporter(unittest.TestCase): - @mock.patch( - "opentelemetry.exporter.opencensus.trace_exporter.trace._TRACER_PROVIDER", - None, - ) +class TestCollectorSpanExporter(TraceGlobalsTest, unittest.TestCase): def test_constructor(self): mock_get_node = mock.Mock() patch = mock.patch( @@ -329,10 +326,6 @@ def test_export(self): getattr(output_identifier, "host_name"), "testHostName" ) - @mock.patch( - "opentelemetry.exporter.opencensus.trace_exporter.trace._TRACER_PROVIDER", - None, - ) def test_export_service_name(self): trace_api.set_tracer_provider( TracerProvider( diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 24c42b04c64..26df821cbc3 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -108,6 +108,7 @@ ) from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util import types +from opentelemetry.util._once import Once from opentelemetry.util._providers import _load_provider logger = getLogger(__name__) @@ -452,8 +453,9 @@ def start_as_current_span( yield INVALID_SPAN -_TRACER_PROVIDER = None -_PROXY_TRACER_PROVIDER = None +_TRACER_PROVIDER_SET_ONCE = Once() +_TRACER_PROVIDER: Optional[TracerProvider] = None +_PROXY_TRACER_PROVIDER = ProxyTracerProvider() def get_tracer( @@ -476,40 +478,40 @@ def get_tracer( ) +def _set_tracer_provider(tracer_provider: TracerProvider, log: bool) -> None: + def set_tp() -> None: + global _TRACER_PROVIDER # pylint: disable=global-statement + _TRACER_PROVIDER = tracer_provider + + did_set = _TRACER_PROVIDER_SET_ONCE.do_once(set_tp) + + if log and not did_set: + logger.warning("Overriding of current TracerProvider is not allowed") + + def set_tracer_provider(tracer_provider: TracerProvider) -> None: """Sets the current global :class:`~.TracerProvider` object. This can only be done once, a warning will be logged if any furter attempt is made. """ - global _TRACER_PROVIDER # pylint: disable=global-statement - - if _TRACER_PROVIDER is not None: - logger.warning("Overriding of current TracerProvider is not allowed") - return - - _TRACER_PROVIDER = tracer_provider + _set_tracer_provider(tracer_provider, log=True) def get_tracer_provider() -> TracerProvider: """Gets the current global :class:`~.TracerProvider` object.""" - # pylint: disable=global-statement - global _TRACER_PROVIDER - global _PROXY_TRACER_PROVIDER - if _TRACER_PROVIDER is None: # if a global tracer provider has not been set either via code or env # vars, return a proxy tracer provider if OTEL_PYTHON_TRACER_PROVIDER not in os.environ: - if not _PROXY_TRACER_PROVIDER: - _PROXY_TRACER_PROVIDER = ProxyTracerProvider() return _PROXY_TRACER_PROVIDER - _TRACER_PROVIDER = cast( # type: ignore - "TracerProvider", - _load_provider(OTEL_PYTHON_TRACER_PROVIDER, "tracer_provider"), + tracer_provider: TracerProvider = _load_provider( + OTEL_PYTHON_TRACER_PROVIDER, "tracer_provider" ) - return _TRACER_PROVIDER + _set_tracer_provider(tracer_provider, log=False) + # _TRACER_PROVIDER will have been set by one thread + return cast("TracerProvider", _TRACER_PROVIDER) @contextmanager # type: ignore diff --git a/opentelemetry-api/src/opentelemetry/util/_once.py b/opentelemetry-api/src/opentelemetry/util/_once.py new file mode 100644 index 00000000000..c0cee43a174 --- /dev/null +++ b/opentelemetry-api/src/opentelemetry/util/_once.py @@ -0,0 +1,47 @@ +# 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. + +from threading import Lock +from typing import Callable + + +class Once: + """Execute a function exactly once and block all callers until the function returns + + Same as golang's `sync.Once `_ + """ + + def __init__(self) -> None: + self._lock = Lock() + self._done = False + + def do_once(self, func: Callable[[], None]) -> bool: + """Execute ``func`` if it hasn't been executed or return. + + Will block until ``func`` has been called by one thread. + + Returns: + Whether or not ``func`` was executed in this call + """ + + # fast path, try to avoid locking + if self._done: + return False + + with self._lock: + if not self._done: + func() + self._done = True + return True + return False diff --git a/opentelemetry-api/tests/trace/test_globals.py b/opentelemetry-api/tests/trace/test_globals.py index 034a97e4ded..421b72d65fe 100644 --- a/opentelemetry-api/tests/trace/test_globals.py +++ b/opentelemetry-api/tests/trace/test_globals.py @@ -1,7 +1,9 @@ import unittest -from unittest.mock import patch +from unittest.mock import Mock, patch from opentelemetry import context, trace +from opentelemetry.test.concurrency_test import ConcurrencyTestBase, MockFunc +from opentelemetry.test.globals_test import TraceGlobalsTest from opentelemetry.trace.status import Status, StatusCode @@ -25,25 +27,60 @@ def record_exception( self.recorded_exception = exception -class TestGlobals(unittest.TestCase): - def setUp(self): - self._patcher = patch("opentelemetry.trace._TRACER_PROVIDER") - self._mock_tracer_provider = self._patcher.start() - - def tearDown(self) -> None: - self._patcher.stop() - - def test_get_tracer(self): +class TestGlobals(TraceGlobalsTest, unittest.TestCase): + @staticmethod + @patch("opentelemetry.trace._TRACER_PROVIDER") + def test_get_tracer(mock_tracer_provider): # type: ignore """trace.get_tracer should proxy to the global tracer provider.""" trace.get_tracer("foo", "var") - self._mock_tracer_provider.get_tracer.assert_called_with( - "foo", "var", None - ) - mock_provider = unittest.mock.Mock() + mock_tracer_provider.get_tracer.assert_called_with("foo", "var", None) + mock_provider = Mock() trace.get_tracer("foo", "var", mock_provider) mock_provider.get_tracer.assert_called_with("foo", "var", None) +class TestGlobalsConcurrency(TraceGlobalsTest, ConcurrencyTestBase): + @patch("opentelemetry.trace.logger") + def test_set_tracer_provider_many_threads(self, mock_logger) -> None: # type: ignore + mock_logger.warning = MockFunc() + + def do_concurrently() -> Mock: + # first get a proxy tracer + proxy_tracer = trace.ProxyTracerProvider().get_tracer("foo") + + # try to set the global tracer provider + mock_tracer_provider = Mock(get_tracer=MockFunc()) + trace.set_tracer_provider(mock_tracer_provider) + + # start a span through the proxy which will call through to the mock provider + proxy_tracer.start_span("foo") + + return mock_tracer_provider + + num_threads = 100 + mock_tracer_providers = self.run_with_many_threads( + do_concurrently, + num_threads=num_threads, + ) + + # despite trying to set tracer provider many times, only one of the + # mock_tracer_providers should have stuck and been called from + # proxy_tracer.start_span() + mock_tps_with_any_call = [ + mock + for mock in mock_tracer_providers + if mock.get_tracer.call_count > 0 + ] + + self.assertEqual(len(mock_tps_with_any_call), 1) + self.assertEqual( + mock_tps_with_any_call[0].get_tracer.call_count, num_threads + ) + + # should have warned everytime except for the successful set + self.assertEqual(mock_logger.warning.call_count, num_threads - 1) + + class TestTracer(unittest.TestCase): def setUp(self): # pylint: disable=protected-access diff --git a/opentelemetry-api/tests/trace/test_proxy.py b/opentelemetry-api/tests/trace/test_proxy.py index 42a31b41322..da1d60c74e1 100644 --- a/opentelemetry-api/tests/trace/test_proxy.py +++ b/opentelemetry-api/tests/trace/test_proxy.py @@ -17,6 +17,7 @@ import unittest from opentelemetry import trace +from opentelemetry.test.globals_test import TraceGlobalsTest from opentelemetry.trace.span import INVALID_SPAN_CONTEXT, NonRecordingSpan @@ -39,10 +40,8 @@ class TestSpan(NonRecordingSpan): pass -class TestProxy(unittest.TestCase): +class TestProxy(TraceGlobalsTest, unittest.TestCase): def test_proxy_tracer(self): - original_provider = trace._TRACER_PROVIDER - provider = trace.get_tracer_provider() # proxy provider self.assertIsInstance(provider, trace.ProxyTracerProvider) @@ -60,6 +59,9 @@ def test_proxy_tracer(self): # set a real provider trace.set_tracer_provider(TestProvider()) + # get_tracer_provider() now returns the real provider + self.assertIsInstance(trace.get_tracer_provider(), TestProvider) + # tracer provider now returns real instance self.assertIsInstance(trace.get_tracer_provider(), TestProvider) @@ -71,5 +73,3 @@ def test_proxy_tracer(self): # creates real spans with tracer.start_span("") as span: self.assertIsInstance(span, TestSpan) - - trace._TRACER_PROVIDER = original_provider diff --git a/opentelemetry-api/tests/util/test_once.py b/opentelemetry-api/tests/util/test_once.py new file mode 100644 index 00000000000..ee94318d228 --- /dev/null +++ b/opentelemetry-api/tests/util/test_once.py @@ -0,0 +1,48 @@ +# 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. + +from opentelemetry.test.concurrency_test import ConcurrencyTestBase, MockFunc +from opentelemetry.util._once import Once + + +class TestOnce(ConcurrencyTestBase): + def test_once_single_thread(self): + once_func = MockFunc() + once = Once() + + self.assertEqual(once_func.call_count, 0) + + # first call should run + called = once.do_once(once_func) + self.assertTrue(called) + self.assertEqual(once_func.call_count, 1) + + # subsequent calls do nothing + called = once.do_once(once_func) + self.assertFalse(called) + self.assertEqual(once_func.call_count, 1) + + def test_once_many_threads(self): + once_func = MockFunc() + once = Once() + + def run_concurrently() -> bool: + return once.do_once(once_func) + + results = self.run_with_many_threads(run_concurrently, num_threads=100) + + self.assertEqual(once_func.call_count, 1) + + # check that only one of the threads got True + self.assertEqual(results.count(True), 1) diff --git a/tests/util/src/opentelemetry/test/concurrency_test.py b/tests/util/src/opentelemetry/test/concurrency_test.py new file mode 100644 index 00000000000..5d178e24fff --- /dev/null +++ b/tests/util/src/opentelemetry/test/concurrency_test.py @@ -0,0 +1,90 @@ +# 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 sys +import threading +import unittest +from functools import partial +from typing import Callable, List, Optional, TypeVar +from unittest.mock import Mock + +ReturnT = TypeVar("ReturnT") + + +class MockFunc: + """A thread safe mock function + + Use this as part of your mock if you want to count calls across multiple + threads. + """ + + def __init__(self) -> None: + self.lock = threading.Lock() + self.call_count = 0 + self.mock = Mock() + + def __call__(self, *args, **kwargs): + with self.lock: + self.call_count += 1 + return self.mock + + +class ConcurrencyTestBase(unittest.TestCase): + """Test base class/mixin for tests of concurrent code + + This test class calls ``sys.setswitchinterval(1e-12)`` to try to create more + contention while running tests that use many threads. It also provides + ``run_with_many_threads`` to run some test code in many threads + concurrently. + """ + + orig_switch_interval = sys.getswitchinterval() + + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + # switch threads more often to increase chance of contention + sys.setswitchinterval(1e-12) + + @classmethod + def tearDownClass(cls) -> None: + super().tearDownClass() + sys.setswitchinterval(cls.orig_switch_interval) + + @staticmethod + def run_with_many_threads( + func_to_test: Callable[[], ReturnT], + num_threads: int = 100, + ) -> List[ReturnT]: + """Util to run ``func_to_test`` in ``num_threads`` concurrently""" + + barrier = threading.Barrier(num_threads) + results: List[Optional[ReturnT]] = [None] * num_threads + + def thread_start(idx: int) -> None: + nonlocal results + # Get all threads here before releasing them to create contention + barrier.wait() + results[idx] = func_to_test() + + threads = [ + threading.Thread(target=partial(thread_start, i)) + for i in range(num_threads) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + + return results # type: ignore diff --git a/tests/util/src/opentelemetry/test/globals_test.py b/tests/util/src/opentelemetry/test/globals_test.py new file mode 100644 index 00000000000..bb2cad6a0ac --- /dev/null +++ b/tests/util/src/opentelemetry/test/globals_test.py @@ -0,0 +1,41 @@ +# 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 unittest + +from opentelemetry import trace as trace_api +from opentelemetry.util._once import Once + + +# pylint: disable=protected-access +def reset_trace_globals() -> None: + """WARNING: only use this for tests.""" + trace_api._TRACER_PROVIDER_SET_ONCE = Once() + trace_api._TRACER_PROVIDER = None + trace_api._PROXY_TRACER_PROVIDER = trace_api.ProxyTracerProvider() + + +class TraceGlobalsTest(unittest.TestCase): + """Resets trace API globals in setUp/tearDown + + Use as a base class or mixin for your test that modifies trace API globals. + """ + + def setUp(self) -> None: + super().setUp() + reset_trace_globals() + + def tearDown(self) -> None: + super().tearDown() + reset_trace_globals() diff --git a/tests/util/src/opentelemetry/test/test_base.py b/tests/util/src/opentelemetry/test/test_base.py index 9762a08010e..f176238add3 100644 --- a/tests/util/src/opentelemetry/test/test_base.py +++ b/tests/util/src/opentelemetry/test/test_base.py @@ -21,6 +21,7 @@ from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( InMemorySpanExporter, ) +from opentelemetry.test.globals_test import reset_trace_globals class TestBase(unittest.TestCase): @@ -28,20 +29,18 @@ class TestBase(unittest.TestCase): @classmethod def setUpClass(cls): - cls.original_tracer_provider = trace_api.get_tracer_provider() result = cls.create_tracer_provider() cls.tracer_provider, cls.memory_exporter = result # This is done because set_tracer_provider cannot override the # current tracer provider. - trace_api._TRACER_PROVIDER = None # pylint: disable=protected-access + reset_trace_globals() trace_api.set_tracer_provider(cls.tracer_provider) @classmethod def tearDownClass(cls): # This is done because set_tracer_provider cannot override the # current tracer provider. - trace_api._TRACER_PROVIDER = None # pylint: disable=protected-access - trace_api.set_tracer_provider(cls.original_tracer_provider) + reset_trace_globals() def setUp(self): self.memory_exporter.clear() diff --git a/tox.ini b/tox.ini index 724a7f20185..0210e7e6e9d 100644 --- a/tox.ini +++ b/tox.ini @@ -85,7 +85,7 @@ setenv = ; i.e: CONTRIB_REPO_SHA=dde62cebffe519c35875af6d06fae053b3be65ec tox -e CONTRIB_REPO_SHA={env:CONTRIB_REPO_SHA:"main"} CONTRIB_REPO="git+https://github.com/open-telemetry/opentelemetry-python-contrib.git@{env:CONTRIB_REPO_SHA}" - mypy: MYPYPATH={toxinidir}/opentelemetry-api/src/ + mypy: MYPYPATH={toxinidir}/opentelemetry-api/src/:{toxinidir}/tests/util/src/ changedir = api: opentelemetry-api/tests From bba4a04df43bc0f4c234d0d4072aa130392eea19 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Fri, 24 Sep 2021 18:17:11 +0200 Subject: [PATCH 11/13] Adds metrics API (#1887) * Adds metric prototype Fixes #1835 * Fix docs * Add API metrics doc * Add missing docs * Add files * Adding docs * Refactor to _initialize * Refactor initialize * Add more documentation * Add exporter test * Add process * Fix tests * Try to add aggregator_class argument Tests are failing here * Fix instrument parent classes * Test default aggregator * WIP * Add prototype test * Tests passing again * Use right counters * All tests passing * Rearrange instrument storage * Fix tests * Add HTTP server test * WIP * WIP * Add prototype * WIP * Fail the test * WIP * WIP * WIP * WIP * Add views * Discard instruments via views * Fix tests * WIP * WIP * Fix lint * WIP * Fix test * Fix lint * Fix method * Fix lint * Mypy workaround * Skip if 3.6 * Fix lint * Add reason * Fix 3.6 * Fix run * Fix lint * Remove SDK metrics * Remove SDK docs * Remove metrics * Remove assertnotraises mixin * Revert sdk docs conf * Remove SDK env var changes * Fix unit checking * Define positional-only arguments * Add Metrics plans * Add API tests * WIP * WIP test * WIP * WIP * WIP * Set provider test passing * Use a fixture * Add test for get_provider * Rename tests * WIP * WIP * WIP * WIP * Remove non specific requirement * Add meter requirements * Put all meter provider tests in one file * Add meter tests * Make attributes be passed as a dictionary * Make some interfaces private * Log an error instead * Remove ASCII flag * Add CHANGELOG entry * Add instrument tests * All tests passing * Add test * Add name tests * Add unit tests * Add description tests * Add counter tests * Add more tests * Add Histogram tests * Add observable gauge tests * Add updowncounter tests * Add observableupdowncounter tests * Fix lint * Fix docs * Fix lint * Ignore mypy * Remove useless pylint skip * Remove useless pylint skip * Remove useless pylint skip * Remove useless pylint skip * Remove useless pylint skip * Add locks to meter and meterprovider * Add lock to instruments * Fix fixmes * Fix lint * Add documentation placeholder * Remove blank line as requested. * Do not override Rlock * Remove unecessary super calls * Add missing super calls * Remove plan files * Add missing parameters * Rename observe to callback * Fix lint * Rename to secure_instrument_name * Remove locks * Fix lint * Remove args and kwargs * Remove implementation that gives meters access to meter provider * Allow creating async instruments with either a callback function or generator * add additional test with callback form of observable counter * add a test/example that reads measurements from proc stat * implement cpu time integration test with generator too Co-authored-by: Aaron Abbott --- CHANGELOG.md | 2 + docs/api/api.rst | 1 + docs/api/metrics.instrument.rst | 8 + docs/api/metrics.measurement.rst | 7 + docs/api/metrics.rst | 15 + .../opentelemetry/environment_variables.py | 11 + .../src/opentelemetry/metrics/__init__.py | 384 +++++++++ .../src/opentelemetry/metrics/instrument.py | 234 +++++ .../src/opentelemetry/metrics/measurement.py | 39 + .../metrics/integration_test/test_cpu_time.py | 194 +++++ .../tests/metrics/test_instruments.py | 804 ++++++++++++++++++ opentelemetry-api/tests/metrics/test_meter.py | 179 ++++ .../tests/metrics/test_meter_provider.py | 260 ++++++ tests/util/src/opentelemetry/test/__init__.py | 0 tox.ini | 2 +- 15 files changed, 2139 insertions(+), 1 deletion(-) create mode 100644 docs/api/metrics.instrument.rst create mode 100644 docs/api/metrics.measurement.rst create mode 100644 docs/api/metrics.rst create mode 100644 opentelemetry-api/src/opentelemetry/metrics/__init__.py create mode 100644 opentelemetry-api/src/opentelemetry/metrics/instrument.py create mode 100644 opentelemetry-api/src/opentelemetry/metrics/measurement.py create mode 100644 opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py create mode 100644 opentelemetry-api/tests/metrics/test_instruments.py create mode 100644 opentelemetry-api/tests/metrics/test_meter.py create mode 100644 opentelemetry-api/tests/metrics/test_meter_provider.py delete mode 100644 tests/util/src/opentelemetry/test/__init__.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 2523b5834d6..1be15da6312 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 ([#2182](https://github.com/open-telemetry/opentelemetry-python/pull/2182)) - Automatically load OTEL environment variables as options for `opentelemetry-instrument` ([#1969](https://github.com/open-telemetry/opentelemetry-python/pull/1969)) +- Add metrics API + ([#1887](https://github.com/open-telemetry/opentelemetry-python/pull/1887)) - `opentelemetry-semantic-conventions` Update to semantic conventions v1.6.1 ([#2077](https://github.com/open-telemetry/opentelemetry-python/pull/2077)) - Do not count invalid attributes for dropped diff --git a/docs/api/api.rst b/docs/api/api.rst index e531d1419ee..a13c9e698bb 100644 --- a/docs/api/api.rst +++ b/docs/api/api.rst @@ -9,4 +9,5 @@ OpenTelemetry Python API baggage context trace + metrics environment_variables diff --git a/docs/api/metrics.instrument.rst b/docs/api/metrics.instrument.rst new file mode 100644 index 00000000000..efceaf74c6d --- /dev/null +++ b/docs/api/metrics.instrument.rst @@ -0,0 +1,8 @@ +opentelemetry.metrics.instrument +================================ + +.. automodule:: opentelemetry.metrics.instrument + :members: + :private-members: + :undoc-members: + :show-inheritance: diff --git a/docs/api/metrics.measurement.rst b/docs/api/metrics.measurement.rst new file mode 100644 index 00000000000..4674169c134 --- /dev/null +++ b/docs/api/metrics.measurement.rst @@ -0,0 +1,7 @@ +opentelemetry.metrics.measurement +================================= + +.. automodule:: opentelemetry.metrics.measurement + :members: + :undoc-members: + :show-inheritance: diff --git a/docs/api/metrics.rst b/docs/api/metrics.rst new file mode 100644 index 00000000000..e445a5015e1 --- /dev/null +++ b/docs/api/metrics.rst @@ -0,0 +1,15 @@ +opentelemetry.metrics package +============================= + +Submodules +---------- + +.. toctree:: + + metrics.instrument + metrics.measurement + +Module contents +--------------- + +.. automodule:: opentelemetry.metrics diff --git a/opentelemetry-api/src/opentelemetry/environment_variables.py b/opentelemetry-api/src/opentelemetry/environment_variables.py index 83ec67149dd..1e2b8f90d35 100644 --- a/opentelemetry-api/src/opentelemetry/environment_variables.py +++ b/opentelemetry-api/src/opentelemetry/environment_variables.py @@ -36,3 +36,14 @@ """ .. envvar:: OTEL_PYTHON_TRACER_PROVIDER """ + +OTEL_PYTHON_METER_PROVIDER = "OTEL_PYTHON_METER_PROVIDER" +""" +.. envvar:: OTEL_PYTHON_METER_PROVIDER +""" + +OTEL_METRICS_EXPORTER = "OTEL_METRICS_EXPORTER" +""" +.. envvar:: OTEL_METRICS_EXPORTER + +""" diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py new file mode 100644 index 00000000000..83d210e063b --- /dev/null +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -0,0 +1,384 @@ +# 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. + +# pylint: disable=too-many-ancestors +# type: ignore + +# FIXME enhance the documentation of this module +""" +This module provides abstract and concrete (but noop) classes that can be used +to generate metrics. +""" + + +from abc import ABC, abstractmethod +from logging import getLogger +from os import environ +from typing import Optional, cast + +from opentelemetry.environment_variables import OTEL_PYTHON_METER_PROVIDER +from opentelemetry.metrics.instrument import ( + Counter, + DefaultCounter, + DefaultHistogram, + DefaultObservableCounter, + DefaultObservableGauge, + DefaultObservableUpDownCounter, + DefaultUpDownCounter, + Histogram, + ObservableCounter, + ObservableGauge, + ObservableUpDownCounter, + UpDownCounter, +) +from opentelemetry.util._providers import _load_provider + +_logger = getLogger(__name__) + + +class MeterProvider(ABC): + @abstractmethod + def get_meter( + self, + name, + version=None, + schema_url=None, + ) -> "Meter": + if name is None or name == "": + _logger.warning("Invalid name: %s", name) + + +class _DefaultMeterProvider(MeterProvider): + def get_meter( + self, + name, + version=None, + schema_url=None, + ) -> "Meter": + super().get_meter(name, version=version, schema_url=schema_url) + return _DefaultMeter(name, version=version, schema_url=schema_url) + + +class ProxyMeterProvider(MeterProvider): + def get_meter( + self, + name, + version=None, + schema_url=None, + ) -> "Meter": + if _METER_PROVIDER: + return _METER_PROVIDER.get_meter( + name, version=version, schema_url=schema_url + ) + return ProxyMeter(name, version=version, schema_url=schema_url) + + +class Meter(ABC): + def __init__(self, name, version=None, schema_url=None): + super().__init__() + self._name = name + self._version = version + self._schema_url = schema_url + self._instrument_names = set() + + @property + def name(self): + return self._name + + @property + def version(self): + return self._version + + @property + def schema_url(self): + return self._schema_url + + def _secure_instrument_name(self, name): + name = name.lower() + + if name in self._instrument_names: + _logger.error("Instrument name %s has been used already", name) + + else: + self._instrument_names.add(name) + + @abstractmethod + def create_counter(self, name, unit="", description="") -> Counter: + self._secure_instrument_name(name) + + @abstractmethod + def create_up_down_counter( + self, name, unit="", description="" + ) -> UpDownCounter: + self._secure_instrument_name(name) + + @abstractmethod + def create_observable_counter( + self, name, callback, unit="", description="" + ) -> ObservableCounter: + """Creates an observable counter instrument + + An observable counter observes a monotonically increasing count by + calling a provided callback which returns multiple + :class:`~opentelemetry.metrics.measurement.Measurement`. + + For example, an observable counter could be used to report system CPU + time periodically. Here is a basic implementation:: + + def cpu_time_callback() -> Iterable[Measurement]: + measurements = [] + with open("/proc/stat") as procstat: + procstat.readline() # skip the first line + for line in procstat: + if not line.startswith("cpu"): break + cpu, *states = line.split() + measurements.append(Measurement(int(states[0]) // 100, {"cpu": cpu, "state": "user"})) + measurements.append(Measurement(int(states[1]) // 100, {"cpu": cpu, "state": "nice"})) + measurements.append(Measurement(int(states[2]) // 100, {"cpu": cpu, "state": "system"})) + # ... other states + return measurements + + meter.create_observable_counter( + "system.cpu.time", + callback=cpu_time_callback, + unit="s", + description="CPU time" + ) + + To reduce memory usage, you can use generator callbacks instead of + building the full list:: + + def cpu_time_callback() -> Iterable[Measurement]: + with open("/proc/stat") as procstat: + procstat.readline() # skip the first line + for line in procstat: + if not line.startswith("cpu"): break + cpu, *states = line.split() + yield Measurement(int(states[0]) // 100, {"cpu": cpu, "state": "user"}) + yield Measurement(int(states[1]) // 100, {"cpu": cpu, "state": "nice"}) + # ... other states + + Alternatively, you can pass a generator directly instead of a callback, + which should return iterables of + :class:`~opentelemetry.metrics.measurement.Measurement`:: + + def cpu_time_callback(states_to_include: set[str]) -> Iterable[Iterable[Measurement]]: + while True: + measurements = [] + with open("/proc/stat") as procstat: + procstat.readline() # skip the first line + for line in procstat: + if not line.startswith("cpu"): break + cpu, *states = line.split() + if "user" in states_to_include: + measurements.append(Measurement(int(states[0]) // 100, {"cpu": cpu, "state": "user"})) + if "nice" in states_to_include: + measurements.append(Measurement(int(states[1]) // 100, {"cpu": cpu, "state": "nice"})) + # ... other states + yield measurements + + meter.create_observable_counter( + "system.cpu.time", + callback=cpu_time_callback({"user", "system"}), + unit="s", + description="CPU time" + ) + + Args: + name: The name of the instrument to be created + callback: A callback that returns an iterable of + :class:`~opentelemetry.metrics.measurement.Measurement`. + Alternatively, can be a generator that yields iterables of + :class:`~opentelemetry.metrics.measurement.Measurement`. + unit: The unit for measurements this instrument reports. For + example, ``By`` for bytes. UCUM units are recommended. + description: A description for this instrument and what it measures. + """ + + self._secure_instrument_name(name) + + @abstractmethod + def create_histogram(self, name, unit="", description="") -> Histogram: + self._secure_instrument_name(name) + + @abstractmethod + def create_observable_gauge( + self, name, callback, unit="", description="" + ) -> ObservableGauge: + self._secure_instrument_name(name) + + @abstractmethod + def create_observable_up_down_counter( + self, name, callback, unit="", description="" + ) -> ObservableUpDownCounter: + self._secure_instrument_name(name) + + +class ProxyMeter(Meter): + def __init__( + self, + name, + version=None, + schema_url=None, + ): + super().__init__(name, version=version, schema_url=schema_url) + self._real_meter: Optional[Meter] = None + self._noop_meter = _DefaultMeter( + name, version=version, schema_url=schema_url + ) + + @property + def _meter(self) -> Meter: + if self._real_meter is not None: + return self._real_meter + + if _METER_PROVIDER: + self._real_meter = _METER_PROVIDER.get_meter( + self._name, + self._version, + ) + return self._real_meter + return self._noop_meter + + def create_counter(self, *args, **kwargs) -> Counter: + return self._meter.create_counter(*args, **kwargs) + + def create_up_down_counter(self, *args, **kwargs) -> UpDownCounter: + return self._meter.create_up_down_counter(*args, **kwargs) + + def create_observable_counter(self, *args, **kwargs) -> ObservableCounter: + return self._meter.create_observable_counter(*args, **kwargs) + + def create_histogram(self, *args, **kwargs) -> Histogram: + return self._meter.create_histogram(*args, **kwargs) + + def create_observable_gauge(self, *args, **kwargs) -> ObservableGauge: + return self._meter.create_observable_gauge(*args, **kwargs) + + def create_observable_up_down_counter( + self, *args, **kwargs + ) -> ObservableUpDownCounter: + return self._meter.create_observable_up_down_counter(*args, **kwargs) + + +class _DefaultMeter(Meter): + def create_counter(self, name, unit="", description="") -> Counter: + super().create_counter(name, unit=unit, description=description) + return DefaultCounter(name, unit=unit, description=description) + + def create_up_down_counter( + self, name, unit="", description="" + ) -> UpDownCounter: + super().create_up_down_counter( + name, unit=unit, description=description + ) + return DefaultUpDownCounter(name, unit=unit, description=description) + + def create_observable_counter( + self, name, callback, unit="", description="" + ) -> ObservableCounter: + super().create_observable_counter( + name, callback, unit=unit, description=description + ) + return DefaultObservableCounter( + name, + callback, + unit=unit, + description=description, + ) + + def create_histogram(self, name, unit="", description="") -> Histogram: + super().create_histogram(name, unit=unit, description=description) + return DefaultHistogram(name, unit=unit, description=description) + + def create_observable_gauge( + self, name, callback, unit="", description="" + ) -> ObservableGauge: + super().create_observable_gauge( + name, callback, unit=unit, description=description + ) + return DefaultObservableGauge( + name, + callback, + unit=unit, + description=description, + ) + + def create_observable_up_down_counter( + self, name, callback, unit="", description="" + ) -> ObservableUpDownCounter: + super().create_observable_up_down_counter( + name, callback, unit=unit, description=description + ) + return DefaultObservableUpDownCounter( + name, + callback, + unit=unit, + description=description, + ) + + +_METER_PROVIDER = None +_PROXY_METER_PROVIDER = None + + +def get_meter( + name: str, + version: str = "", + meter_provider: Optional[MeterProvider] = None, +) -> "Meter": + """Returns a `Meter` for use by the given instrumentation library. + + This function is a convenience wrapper for + opentelemetry.trace.MeterProvider.get_meter. + + If meter_provider is omitted the current configured one is used. + """ + if meter_provider is None: + meter_provider = get_meter_provider() + return meter_provider.get_meter(name, version) + + +def set_meter_provider(meter_provider: MeterProvider) -> None: + """Sets the current global :class:`~.MeterProvider` object. + + This can only be done once, a warning will be logged if any furter attempt + is made. + """ + global _METER_PROVIDER # pylint: disable=global-statement + + if _METER_PROVIDER is not None: + _logger.warning("Overriding of current MeterProvider is not allowed") + return + + _METER_PROVIDER = meter_provider + + +def get_meter_provider() -> MeterProvider: + """Gets the current global :class:`~.MeterProvider` object.""" + # pylint: disable=global-statement + global _METER_PROVIDER + global _PROXY_METER_PROVIDER + + if _METER_PROVIDER is None: + if OTEL_PYTHON_METER_PROVIDER not in environ.keys(): + if _PROXY_METER_PROVIDER is None: + _PROXY_METER_PROVIDER = ProxyMeterProvider() + return _PROXY_METER_PROVIDER + + _METER_PROVIDER = cast( + "MeterProvider", + _load_provider(OTEL_PYTHON_METER_PROVIDER, "meter_provider"), + ) + return _METER_PROVIDER diff --git a/opentelemetry-api/src/opentelemetry/metrics/instrument.py b/opentelemetry-api/src/opentelemetry/metrics/instrument.py new file mode 100644 index 00000000000..5d382056408 --- /dev/null +++ b/opentelemetry-api/src/opentelemetry/metrics/instrument.py @@ -0,0 +1,234 @@ +# 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. + +# pylint: disable=too-many-ancestors + +# type: ignore + +from abc import ABC, abstractmethod +from collections import abc as collections_abc +from logging import getLogger +from re import compile as compile_ +from typing import Callable, Generator, Iterable, Union + +from opentelemetry.metrics.measurement import Measurement + +_TInstrumentCallback = Callable[[], Iterable[Measurement]] +_TInstrumentCallbackGenerator = Generator[Iterable[Measurement], None, None] +TCallback = Union[_TInstrumentCallback, _TInstrumentCallbackGenerator] + + +_logger = getLogger(__name__) + + +class Instrument(ABC): + + _name_regex = compile_(r"[a-zA-Z][-.\w]{0,62}") + + @property + def name(self): + return self._name + + @property + def unit(self): + return self._unit + + @property + def description(self): + return self._description + + @abstractmethod + def __init__(self, name, unit="", description=""): + + if name is None or self._name_regex.fullmatch(name) is None: + _logger.error("Invalid instrument name %s", name) + + else: + self._name = name + + if unit is None: + self._unit = "" + elif len(unit) > 63: + _logger.error("unit must be 63 characters or shorter") + + elif any(ord(character) > 127 for character in unit): + _logger.error("unit must only contain ASCII characters") + else: + self._unit = unit + + if description is None: + description = "" + + self._description = description + + +class Synchronous(Instrument): + pass + + +class Asynchronous(Instrument): + @abstractmethod + def __init__( + self, + name, + callback: TCallback, + *args, + unit="", + description="", + **kwargs + ): + super().__init__( + name, *args, unit=unit, description=description, **kwargs + ) + + if isinstance(callback, collections_abc.Callable): + self._callback = callback + elif isinstance(callback, collections_abc.Generator): + self._callback = self._wrap_generator_callback(callback) + else: + _logger.error("callback must be a callable or generator") + + def _wrap_generator_callback( + self, + generator_callback: _TInstrumentCallbackGenerator, + ) -> _TInstrumentCallback: + """Wraps a generator style callback into a callable one""" + has_items = True + + def inner() -> Iterable[Measurement]: + nonlocal has_items + if not has_items: + return [] + + try: + return next(generator_callback) + except StopIteration: + has_items = False + _logger.error( + "callback generator for instrument %s ran out of measurements", + self._name, + ) + return [] + + return inner + + def callback(self): + measurements = self._callback() + if not isinstance(measurements, collections_abc.Iterable): + _logger.error( + "Callback must return an iterable of Measurement, got %s", + type(measurements), + ) + return + for measurement in measurements: + if not isinstance(measurement, Measurement): + _logger.error( + "Callback must return an iterable of Measurement, " + "iterable contained type %s", + type(measurement), + ) + yield measurement + + +class _Adding(Instrument): + pass + + +class _Grouping(Instrument): + pass + + +class _Monotonic(_Adding): + pass + + +class _NonMonotonic(_Adding): + pass + + +class Counter(_Monotonic, Synchronous): + @abstractmethod + def add(self, amount, attributes=None): + if amount < 0: + _logger.error("Amount must be non-negative") + + +class DefaultCounter(Counter): + def __init__(self, name, unit="", description=""): + super().__init__(name, unit=unit, description=description) + + def add(self, amount, attributes=None): + return super().add(amount, attributes=attributes) + + +class UpDownCounter(_NonMonotonic, Synchronous): + @abstractmethod + def add(self, amount, attributes=None): + pass + + +class DefaultUpDownCounter(UpDownCounter): + def __init__(self, name, unit="", description=""): + super().__init__(name, unit=unit, description=description) + + def add(self, amount, attributes=None): + return super().add(amount, attributes=attributes) + + +class ObservableCounter(_Monotonic, Asynchronous): + def callback(self): + measurements = super().callback() + + for measurement in measurements: + if measurement.value < 0: + _logger.error("Amount must be non-negative") + yield measurement + + +class DefaultObservableCounter(ObservableCounter): + def __init__(self, name, callback, unit="", description=""): + super().__init__(name, callback, unit=unit, description=description) + + +class ObservableUpDownCounter(_NonMonotonic, Asynchronous): + + pass + + +class DefaultObservableUpDownCounter(ObservableUpDownCounter): + def __init__(self, name, callback, unit="", description=""): + super().__init__(name, callback, unit=unit, description=description) + + +class Histogram(_Grouping, Synchronous): + @abstractmethod + def record(self, amount, attributes=None): + pass + + +class DefaultHistogram(Histogram): + def __init__(self, name, unit="", description=""): + super().__init__(name, unit=unit, description=description) + + def record(self, amount, attributes=None): + return super().record(amount, attributes=attributes) + + +class ObservableGauge(_Grouping, Asynchronous): + pass + + +class DefaultObservableGauge(ObservableGauge): + def __init__(self, name, callback, unit="", description=""): + super().__init__(name, callback, unit=unit, description=description) diff --git a/opentelemetry-api/src/opentelemetry/metrics/measurement.py b/opentelemetry-api/src/opentelemetry/metrics/measurement.py new file mode 100644 index 00000000000..6b5b081c266 --- /dev/null +++ b/opentelemetry-api/src/opentelemetry/metrics/measurement.py @@ -0,0 +1,39 @@ +# 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. + +# pylint: disable=too-many-ancestors +# type:ignore + + +from abc import ABC, abstractmethod + + +class Measurement(ABC): + @property + def value(self): + return self._value + + @property + def attributes(self): + return self._attributes + + @abstractmethod + def __init__(self, value, attributes=None): + self._value = value + self._attributes = attributes + + +class DefaultMeasurement(Measurement): + def __init__(self, value, attributes=None): + super().__init__(value, attributes=attributes) diff --git a/opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py b/opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py new file mode 100644 index 00000000000..347f6c4dc48 --- /dev/null +++ b/opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py @@ -0,0 +1,194 @@ +# 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. +# type: ignore + +import io +from typing import Generator, Iterable +from unittest import TestCase + +from opentelemetry.metrics import _DefaultMeter +from opentelemetry.metrics.measurement import Measurement + +# FIXME Test that the instrument methods can be called concurrently safely. + + +class ChildMeasurement(Measurement): + def __init__(self, value, attributes=None): + super().__init__(value, attributes=attributes) + + def __eq__(self, o: Measurement) -> bool: + return self.value == o.value and self.attributes == o.attributes + + +class TestCpuTimeIntegration(TestCase): + """Integration test of scraping CPU time from proc stat with an observable + counter""" + + procstat_str = """\ +cpu 8549517 4919096 9165935 1430260740 1641349 0 1646147 623279 0 0 +cpu0 615029 317746 594601 89126459 129629 0 834346 42137 0 0 +cpu1 588232 349185 640492 89156411 124485 0 241004 41862 0 0 +intr 4370168813 38 9 0 0 1639 0 0 0 0 0 2865202 0 152 0 0 0 0 0 0 0 0 0 0 0 0 7236812 5966240 4501046 6467792 7289114 6048205 5299600 5178254 4642580 6826812 6880917 6230308 6307699 4699637 6119330 4905094 5644039 4700633 10539029 5365438 6086908 2227906 5094323 9685701 10137610 7739951 7143508 8123281 4968458 5683103 9890878 4466603 0 0 0 8929628 0 5 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 +ctxt 6877594077 +btime 1631501040 +processes 2557351 +procs_running 2 +procs_blocked 0 +softirq 1644603067 0 166540056 208 309152755 8936439 0 1354908 935642970 13 222975718\n""" + + measurements_expected = [ + ChildMeasurement(6150, {"cpu": "cpu0", "state": "user"}), + ChildMeasurement(3177, {"cpu": "cpu0", "state": "nice"}), + ChildMeasurement(5946, {"cpu": "cpu0", "state": "system"}), + ChildMeasurement(891264, {"cpu": "cpu0", "state": "idle"}), + ChildMeasurement(1296, {"cpu": "cpu0", "state": "iowait"}), + ChildMeasurement(0, {"cpu": "cpu0", "state": "irq"}), + ChildMeasurement(8343, {"cpu": "cpu0", "state": "softirq"}), + ChildMeasurement(421, {"cpu": "cpu0", "state": "guest"}), + ChildMeasurement(0, {"cpu": "cpu0", "state": "guest_nice"}), + ChildMeasurement(5882, {"cpu": "cpu1", "state": "user"}), + ChildMeasurement(3491, {"cpu": "cpu1", "state": "nice"}), + ChildMeasurement(6404, {"cpu": "cpu1", "state": "system"}), + ChildMeasurement(891564, {"cpu": "cpu1", "state": "idle"}), + ChildMeasurement(1244, {"cpu": "cpu1", "state": "iowait"}), + ChildMeasurement(0, {"cpu": "cpu1", "state": "irq"}), + ChildMeasurement(2410, {"cpu": "cpu1", "state": "softirq"}), + ChildMeasurement(418, {"cpu": "cpu1", "state": "guest"}), + ChildMeasurement(0, {"cpu": "cpu1", "state": "guest_nice"}), + ] + + def test_cpu_time_callback(self): + meter = _DefaultMeter("foo") + + def cpu_time_callback() -> Iterable[Measurement]: + procstat = io.StringIO(self.procstat_str) + procstat.readline() # skip the first line + for line in procstat: + if not line.startswith("cpu"): + break + cpu, *states = line.split() + yield ChildMeasurement( + int(states[0]) // 100, {"cpu": cpu, "state": "user"} + ) + yield ChildMeasurement( + int(states[1]) // 100, {"cpu": cpu, "state": "nice"} + ) + yield ChildMeasurement( + int(states[2]) // 100, {"cpu": cpu, "state": "system"} + ) + yield ChildMeasurement( + int(states[3]) // 100, {"cpu": cpu, "state": "idle"} + ) + yield ChildMeasurement( + int(states[4]) // 100, {"cpu": cpu, "state": "iowait"} + ) + yield ChildMeasurement( + int(states[5]) // 100, {"cpu": cpu, "state": "irq"} + ) + yield ChildMeasurement( + int(states[6]) // 100, {"cpu": cpu, "state": "softirq"} + ) + yield ChildMeasurement( + int(states[7]) // 100, {"cpu": cpu, "state": "guest"} + ) + yield ChildMeasurement( + int(states[8]) // 100, {"cpu": cpu, "state": "guest_nice"} + ) + + observable_counter = meter.create_observable_counter( + "system.cpu.time", + callback=cpu_time_callback, + unit="s", + description="CPU time", + ) + measurements = list(observable_counter.callback()) + self.assertEqual(measurements, self.measurements_expected) + + def test_cpu_time_generator(self): + meter = _DefaultMeter("foo") + + def cpu_time_generator() -> Generator[ + Iterable[Measurement], None, None + ]: + while True: + measurements = [] + procstat = io.StringIO(self.procstat_str) + procstat.readline() # skip the first line + for line in procstat: + if not line.startswith("cpu"): + break + cpu, *states = line.split() + measurements.append( + ChildMeasurement( + int(states[0]) // 100, + {"cpu": cpu, "state": "user"}, + ) + ) + measurements.append( + ChildMeasurement( + int(states[1]) // 100, + {"cpu": cpu, "state": "nice"}, + ) + ) + measurements.append( + ChildMeasurement( + int(states[2]) // 100, + {"cpu": cpu, "state": "system"}, + ) + ) + measurements.append( + ChildMeasurement( + int(states[3]) // 100, + {"cpu": cpu, "state": "idle"}, + ) + ) + measurements.append( + ChildMeasurement( + int(states[4]) // 100, + {"cpu": cpu, "state": "iowait"}, + ) + ) + measurements.append( + ChildMeasurement( + int(states[5]) // 100, {"cpu": cpu, "state": "irq"} + ) + ) + measurements.append( + ChildMeasurement( + int(states[6]) // 100, + {"cpu": cpu, "state": "softirq"}, + ) + ) + measurements.append( + ChildMeasurement( + int(states[7]) // 100, + {"cpu": cpu, "state": "guest"}, + ) + ) + measurements.append( + ChildMeasurement( + int(states[8]) // 100, + {"cpu": cpu, "state": "guest_nice"}, + ) + ) + yield measurements + + observable_counter = meter.create_observable_counter( + "system.cpu.time", + callback=cpu_time_generator(), + unit="s", + description="CPU time", + ) + measurements = list(observable_counter.callback()) + self.assertEqual(measurements, self.measurements_expected) diff --git a/opentelemetry-api/tests/metrics/test_instruments.py b/opentelemetry-api/tests/metrics/test_instruments.py new file mode 100644 index 00000000000..2dd100c9ed7 --- /dev/null +++ b/opentelemetry-api/tests/metrics/test_instruments.py @@ -0,0 +1,804 @@ +# 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. +# type: ignore + +from inspect import Signature, isabstract, signature +from logging import ERROR +from unittest import TestCase + +from opentelemetry.metrics import Meter, _DefaultMeter +from opentelemetry.metrics.instrument import ( + Counter, + DefaultCounter, + DefaultHistogram, + DefaultObservableCounter, + DefaultObservableGauge, + DefaultObservableUpDownCounter, + DefaultUpDownCounter, + Histogram, + Instrument, + ObservableCounter, + ObservableGauge, + ObservableUpDownCounter, + UpDownCounter, +) +from opentelemetry.metrics.measurement import Measurement + +# FIXME Test that the instrument methods can be called concurrently safely. + + +class ChildInstrument(Instrument): + def __init__(self, name, *args, unit="", description="", **kwargs): + super().__init__( + name, *args, unit=unit, description=description, **kwargs + ) + + +class ChildMeasurement(Measurement): + def __init__(self, value, attributes=None): + super().__init__(value, attributes=attributes) + + +class TestInstrument(TestCase): + def test_instrument_has_name(self): + """ + Test that the instrument has name. + """ + + init_signature = signature(Instrument.__init__) + self.assertIn("name", init_signature.parameters.keys()) + self.assertIs( + init_signature.parameters["name"].default, Signature.empty + ) + + self.assertTrue(hasattr(Instrument, "name")) + + def test_instrument_has_unit(self): + """ + Test that the instrument has unit. + """ + + init_signature = signature(Instrument.__init__) + self.assertIn("unit", init_signature.parameters.keys()) + self.assertIs(init_signature.parameters["unit"].default, "") + + self.assertTrue(hasattr(Instrument, "unit")) + + def test_instrument_has_description(self): + """ + Test that the instrument has description. + """ + + init_signature = signature(Instrument.__init__) + self.assertIn("description", init_signature.parameters.keys()) + self.assertIs(init_signature.parameters["description"].default, "") + + self.assertTrue(hasattr(Instrument, "description")) + + def test_instrument_name_syntax(self): + """ + Test that instrument names conform to the specified syntax. + """ + + with self.assertLogs(level=ERROR): + ChildInstrument("") + + with self.assertLogs(level=ERROR): + ChildInstrument(None) + + with self.assertLogs(level=ERROR): + ChildInstrument("1a") + + with self.assertLogs(level=ERROR): + ChildInstrument("_a") + + with self.assertLogs(level=ERROR): + ChildInstrument("!a ") + + with self.assertLogs(level=ERROR): + ChildInstrument("a ") + + with self.assertLogs(level=ERROR): + ChildInstrument("a%") + + with self.assertLogs(level=ERROR): + ChildInstrument("a" * 64) + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + ChildInstrument("abc_def_ghi") + + def test_instrument_unit_syntax(self): + """ + Test that instrument unit conform to the specified syntax. + """ + + with self.assertLogs(level=ERROR): + ChildInstrument("name", unit="a" * 64) + + with self.assertLogs(level=ERROR): + ChildInstrument("name", unit="ñ") + + child_instrument = ChildInstrument("name", unit="a") + self.assertEqual(child_instrument.unit, "a") + + child_instrument = ChildInstrument("name", unit="A") + self.assertEqual(child_instrument.unit, "A") + + child_instrument = ChildInstrument("name") + self.assertEqual(child_instrument.unit, "") + + child_instrument = ChildInstrument("name", unit=None) + self.assertEqual(child_instrument.unit, "") + + def test_instrument_description_syntax(self): + """ + Test that instrument description conform to the specified syntax. + """ + + child_instrument = ChildInstrument("name", description="a") + self.assertEqual(child_instrument.description, "a") + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + ChildInstrument("name", description="a" * 1024) + + child_instrument = ChildInstrument("name") + self.assertEqual(child_instrument.description, "") + + child_instrument = ChildInstrument("name", description=None) + self.assertEqual(child_instrument.description, "") + + +class TestCounter(TestCase): + def test_create_counter(self): + """ + Test that the Counter can be created with create_counter. + """ + + self.assertTrue( + isinstance(_DefaultMeter("name").create_counter("name"), Counter) + ) + + def test_api_counter_abstract(self): + """ + Test that the API Counter is an abstract class. + """ + + self.assertTrue(isabstract(Counter)) + + def test_create_counter_api(self): + """ + Test that the API for creating a counter accepts the name of the instrument. + Test that the API for creating a counter accepts the unit of the instrument. + Test that the API for creating a counter accepts the description of the + """ + + create_counter_signature = signature(Meter.create_counter) + self.assertIn("name", create_counter_signature.parameters.keys()) + self.assertIs( + create_counter_signature.parameters["name"].default, + Signature.empty, + ) + + create_counter_signature = signature(Meter.create_counter) + self.assertIn("unit", create_counter_signature.parameters.keys()) + self.assertIs(create_counter_signature.parameters["unit"].default, "") + + create_counter_signature = signature(Meter.create_counter) + self.assertIn( + "description", create_counter_signature.parameters.keys() + ) + self.assertIs( + create_counter_signature.parameters["description"].default, "" + ) + + def test_counter_add_method(self): + """ + Test that the counter has an add method. + Test that the add method returns None. + Test that the add method accepts optional attributes. + Test that the add method accepts the increment amount. + Test that the add method accepts only positive amounts. + """ + + self.assertTrue(hasattr(Counter, "add")) + + self.assertIsNone(DefaultCounter("name").add(1)) + + add_signature = signature(Counter.add) + self.assertIn("attributes", add_signature.parameters.keys()) + self.assertIs(add_signature.parameters["attributes"].default, None) + + self.assertIn("amount", add_signature.parameters.keys()) + self.assertIs( + add_signature.parameters["amount"].default, Signature.empty + ) + + with self.assertLogs(level=ERROR): + DefaultCounter("name").add(-1) + + +class TestObservableCounter(TestCase): + def test_create_observable_counter(self): + """ + Test that the ObservableCounter can be created with create_observable_counter. + """ + + def callback(): + yield + + self.assertTrue( + isinstance( + _DefaultMeter("name").create_observable_counter( + "name", callback() + ), + ObservableCounter, + ) + ) + + def test_api_observable_counter_abstract(self): + """ + Test that the API ObservableCounter is an abstract class. + """ + + self.assertTrue(isabstract(ObservableCounter)) + + def test_create_observable_counter_api(self): + """ + Test that the API for creating a observable_counter accepts the name of the instrument. + Test that the API for creating a observable_counter accepts a callback. + Test that the API for creating a observable_counter accepts the unit of the instrument. + Test that the API for creating a observable_counter accepts the description of the instrument + """ + + create_observable_counter_signature = signature( + Meter.create_observable_counter + ) + self.assertIn( + "name", create_observable_counter_signature.parameters.keys() + ) + self.assertIs( + create_observable_counter_signature.parameters["name"].default, + Signature.empty, + ) + create_observable_counter_signature = signature( + Meter.create_observable_counter + ) + self.assertIn( + "callback", create_observable_counter_signature.parameters.keys() + ) + self.assertIs( + create_observable_counter_signature.parameters["callback"].default, + Signature.empty, + ) + create_observable_counter_signature = signature( + Meter.create_observable_counter + ) + self.assertIn( + "unit", create_observable_counter_signature.parameters.keys() + ) + self.assertIs( + create_observable_counter_signature.parameters["unit"].default, "" + ) + + create_observable_counter_signature = signature( + Meter.create_observable_counter + ) + self.assertIn( + "description", + create_observable_counter_signature.parameters.keys(), + ) + self.assertIs( + create_observable_counter_signature.parameters[ + "description" + ].default, + "", + ) + + def test_observable_counter_generator(self): + """ + Test that the API for creating a asynchronous counter accepts a generator. + Test that the generator function reports iterable of measurements. + Test that there is a way to pass state to the generator. + Test that the instrument accepts positive measurements. + Test that the instrument does not accept negative measurements. + """ + + create_observable_counter_signature = signature( + Meter.create_observable_counter + ) + self.assertIn( + "callback", create_observable_counter_signature.parameters.keys() + ) + self.assertIs( + create_observable_counter_signature.parameters["name"].default, + Signature.empty, + ) + + def callback(): + yield 1 + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + observable_counter = DefaultObservableCounter( + "name", callback() + ) + + with self.assertLogs(level=ERROR): + # use list() to consume the whole generator returned by callback() + list(observable_counter.callback()) + + def callback(): + yield [ChildMeasurement(1), ChildMeasurement(2)] + yield [ChildMeasurement(-1)] + + observable_counter = DefaultObservableCounter("name", callback()) + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + list(observable_counter.callback()) + + with self.assertLogs(level=ERROR): + list(observable_counter.callback()) + + # out of items in generator, should log once + with self.assertLogs(level=ERROR): + list(observable_counter.callback()) + + # but log only once + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + list(observable_counter.callback()) + + def test_observable_counter_callback(self): + """ + Equivalent to test_observable_counter_generator but uses the callback + form. + """ + + def callback_invalid_return(): + return 1 + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + observable_counter = DefaultObservableCounter( + "name", callback_invalid_return + ) + + with self.assertLogs(level=ERROR): + # use list() to consume the whole generator returned by callback() + list(observable_counter.callback()) + + def callback_valid(): + return [ChildMeasurement(1), ChildMeasurement(2)] + + observable_counter = DefaultObservableCounter("name", callback_valid) + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + list(observable_counter.callback()) + + def callback_one_invalid(): + return [ChildMeasurement(1), ChildMeasurement(-2)] + + observable_counter = DefaultObservableCounter( + "name", callback_one_invalid + ) + + with self.assertLogs(level=ERROR): + list(observable_counter.callback()) + + +class TestHistogram(TestCase): + def test_create_histogram(self): + """ + Test that the Histogram can be created with create_histogram. + """ + + self.assertTrue( + isinstance( + _DefaultMeter("name").create_histogram("name"), Histogram + ) + ) + + def test_api_histogram_abstract(self): + """ + Test that the API Histogram is an abstract class. + """ + + self.assertTrue(isabstract(Histogram)) + + def test_create_histogram_api(self): + """ + Test that the API for creating a histogram accepts the name of the instrument. + Test that the API for creating a histogram accepts the unit of the instrument. + Test that the API for creating a histogram accepts the description of the + """ + + create_histogram_signature = signature(Meter.create_histogram) + self.assertIn("name", create_histogram_signature.parameters.keys()) + self.assertIs( + create_histogram_signature.parameters["name"].default, + Signature.empty, + ) + + create_histogram_signature = signature(Meter.create_histogram) + self.assertIn("unit", create_histogram_signature.parameters.keys()) + self.assertIs( + create_histogram_signature.parameters["unit"].default, "" + ) + + create_histogram_signature = signature(Meter.create_histogram) + self.assertIn( + "description", create_histogram_signature.parameters.keys() + ) + self.assertIs( + create_histogram_signature.parameters["description"].default, "" + ) + + def test_histogram_record_method(self): + """ + Test that the histogram has an record method. + Test that the record method returns None. + Test that the record method accepts optional attributes. + Test that the record method accepts the increment amount. + Test that the record method returns None. + """ + + self.assertTrue(hasattr(Histogram, "record")) + + self.assertIsNone(DefaultHistogram("name").record(1)) + + record_signature = signature(Histogram.record) + self.assertIn("attributes", record_signature.parameters.keys()) + self.assertIs(record_signature.parameters["attributes"].default, None) + + self.assertIn("amount", record_signature.parameters.keys()) + self.assertIs( + record_signature.parameters["amount"].default, Signature.empty + ) + + self.assertIsNone(DefaultHistogram("name").record(1)) + + +class TestObservableGauge(TestCase): + def test_create_observable_gauge(self): + """ + Test that the ObservableGauge can be created with create_observable_gauge. + """ + + def callback(): + yield + + self.assertTrue( + isinstance( + _DefaultMeter("name").create_observable_gauge( + "name", callback() + ), + ObservableGauge, + ) + ) + + def test_api_observable_gauge_abstract(self): + """ + Test that the API ObservableGauge is an abstract class. + """ + + self.assertTrue(isabstract(ObservableGauge)) + + def test_create_observable_gauge_api(self): + """ + Test that the API for creating a observable_gauge accepts the name of the instrument. + Test that the API for creating a observable_gauge accepts a callback. + Test that the API for creating a observable_gauge accepts the unit of the instrument. + Test that the API for creating a observable_gauge accepts the description of the instrument + """ + + create_observable_gauge_signature = signature( + Meter.create_observable_gauge + ) + self.assertIn( + "name", create_observable_gauge_signature.parameters.keys() + ) + self.assertIs( + create_observable_gauge_signature.parameters["name"].default, + Signature.empty, + ) + create_observable_gauge_signature = signature( + Meter.create_observable_gauge + ) + self.assertIn( + "callback", create_observable_gauge_signature.parameters.keys() + ) + self.assertIs( + create_observable_gauge_signature.parameters["callback"].default, + Signature.empty, + ) + create_observable_gauge_signature = signature( + Meter.create_observable_gauge + ) + self.assertIn( + "unit", create_observable_gauge_signature.parameters.keys() + ) + self.assertIs( + create_observable_gauge_signature.parameters["unit"].default, "" + ) + + create_observable_gauge_signature = signature( + Meter.create_observable_gauge + ) + self.assertIn( + "description", create_observable_gauge_signature.parameters.keys() + ) + self.assertIs( + create_observable_gauge_signature.parameters[ + "description" + ].default, + "", + ) + + def test_observable_gauge_callback(self): + """ + Test that the API for creating a asynchronous gauge accepts a callback. + Test that the callback function reports measurements. + Test that there is a way to pass state to the callback. + """ + + create_observable_gauge_signature = signature( + Meter.create_observable_gauge + ) + self.assertIn( + "callback", create_observable_gauge_signature.parameters.keys() + ) + self.assertIs( + create_observable_gauge_signature.parameters["name"].default, + Signature.empty, + ) + + def callback(): + yield + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + observable_gauge = DefaultObservableGauge("name", callback()) + + with self.assertLogs(level=ERROR): + list(observable_gauge.callback()) + + def callback(): + yield [ChildMeasurement(1), ChildMeasurement(-1)] + + observable_gauge = DefaultObservableGauge("name", callback()) + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + list(observable_gauge.callback()) + + +class TestUpDownCounter(TestCase): + def test_create_up_down_counter(self): + """ + Test that the UpDownCounter can be created with create_up_down_counter. + """ + + self.assertTrue( + isinstance( + _DefaultMeter("name").create_up_down_counter("name"), + UpDownCounter, + ) + ) + + def test_api_up_down_counter_abstract(self): + """ + Test that the API UpDownCounter is an abstract class. + """ + + self.assertTrue(isabstract(UpDownCounter)) + + def test_create_up_down_counter_api(self): + """ + Test that the API for creating a up_down_counter accepts the name of the instrument. + Test that the API for creating a up_down_counter accepts the unit of the instrument. + Test that the API for creating a up_down_counter accepts the description of the + """ + + create_up_down_counter_signature = signature( + Meter.create_up_down_counter + ) + self.assertIn( + "name", create_up_down_counter_signature.parameters.keys() + ) + self.assertIs( + create_up_down_counter_signature.parameters["name"].default, + Signature.empty, + ) + + create_up_down_counter_signature = signature( + Meter.create_up_down_counter + ) + self.assertIn( + "unit", create_up_down_counter_signature.parameters.keys() + ) + self.assertIs( + create_up_down_counter_signature.parameters["unit"].default, "" + ) + + create_up_down_counter_signature = signature( + Meter.create_up_down_counter + ) + self.assertIn( + "description", create_up_down_counter_signature.parameters.keys() + ) + self.assertIs( + create_up_down_counter_signature.parameters["description"].default, + "", + ) + + def test_up_down_counter_add_method(self): + """ + Test that the up_down_counter has an add method. + Test that the add method returns None. + Test that the add method accepts optional attributes. + Test that the add method accepts the increment or decrement amount. + Test that the add method accepts positive and negative amounts. + """ + + self.assertTrue(hasattr(UpDownCounter, "add")) + + self.assertIsNone(DefaultUpDownCounter("name").add(1)) + + add_signature = signature(UpDownCounter.add) + self.assertIn("attributes", add_signature.parameters.keys()) + self.assertIs(add_signature.parameters["attributes"].default, None) + + self.assertIn("amount", add_signature.parameters.keys()) + self.assertIs( + add_signature.parameters["amount"].default, Signature.empty + ) + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + DefaultUpDownCounter("name").add(-1) + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + DefaultUpDownCounter("name").add(1) + + +class TestObservableUpDownCounter(TestCase): + def test_create_observable_up_down_counter(self): + """ + Test that the ObservableUpDownCounter can be created with create_observable_up_down_counter. + """ + + def callback(): + yield + + self.assertTrue( + isinstance( + _DefaultMeter("name").create_observable_up_down_counter( + "name", callback() + ), + ObservableUpDownCounter, + ) + ) + + def test_api_observable_up_down_counter_abstract(self): + """ + Test that the API ObservableUpDownCounter is an abstract class. + """ + + self.assertTrue(isabstract(ObservableUpDownCounter)) + + def test_create_observable_up_down_counter_api(self): + """ + Test that the API for creating a observable_up_down_counter accepts the name of the instrument. + Test that the API for creating a observable_up_down_counter accepts a callback. + Test that the API for creating a observable_up_down_counter accepts the unit of the instrument. + Test that the API for creating a observable_up_down_counter accepts the description of the instrument + """ + + create_observable_up_down_counter_signature = signature( + Meter.create_observable_up_down_counter + ) + self.assertIn( + "name", + create_observable_up_down_counter_signature.parameters.keys(), + ) + self.assertIs( + create_observable_up_down_counter_signature.parameters[ + "name" + ].default, + Signature.empty, + ) + create_observable_up_down_counter_signature = signature( + Meter.create_observable_up_down_counter + ) + self.assertIn( + "callback", + create_observable_up_down_counter_signature.parameters.keys(), + ) + self.assertIs( + create_observable_up_down_counter_signature.parameters[ + "callback" + ].default, + Signature.empty, + ) + create_observable_up_down_counter_signature = signature( + Meter.create_observable_up_down_counter + ) + self.assertIn( + "unit", + create_observable_up_down_counter_signature.parameters.keys(), + ) + self.assertIs( + create_observable_up_down_counter_signature.parameters[ + "unit" + ].default, + "", + ) + + create_observable_up_down_counter_signature = signature( + Meter.create_observable_up_down_counter + ) + self.assertIn( + "description", + create_observable_up_down_counter_signature.parameters.keys(), + ) + self.assertIs( + create_observable_up_down_counter_signature.parameters[ + "description" + ].default, + "", + ) + + def test_observable_up_down_counter_callback(self): + """ + Test that the API for creating a asynchronous up_down_counter accepts a callback. + Test that the callback function reports measurements. + Test that there is a way to pass state to the callback. + Test that the instrument accepts positive and negative values. + """ + + create_observable_up_down_counter_signature = signature( + Meter.create_observable_up_down_counter + ) + self.assertIn( + "callback", + create_observable_up_down_counter_signature.parameters.keys(), + ) + self.assertIs( + create_observable_up_down_counter_signature.parameters[ + "name" + ].default, + Signature.empty, + ) + + def callback(): + yield ChildMeasurement(1) + yield ChildMeasurement(-1) + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + observable_up_down_counter = DefaultObservableUpDownCounter( + "name", callback() + ) + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + observable_up_down_counter.callback() + + with self.assertRaises(AssertionError): + with self.assertLogs(level=ERROR): + observable_up_down_counter.callback() diff --git a/opentelemetry-api/tests/metrics/test_meter.py b/opentelemetry-api/tests/metrics/test_meter.py new file mode 100644 index 00000000000..96543b69fe2 --- /dev/null +++ b/opentelemetry-api/tests/metrics/test_meter.py @@ -0,0 +1,179 @@ +# 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. +# type: ignore + +from logging import WARNING +from unittest import TestCase +from unittest.mock import Mock + +from opentelemetry.metrics import Meter + +# FIXME Test that the meter methods can be called concurrently safely. + + +class ChildMeter(Meter): + def create_counter(self, name, unit="", description=""): + super().create_counter(name, unit=unit, description=description) + + def create_up_down_counter(self, name, unit="", description=""): + super().create_up_down_counter( + name, unit=unit, description=description + ) + + def create_observable_counter( + self, name, callback, unit="", description="" + ): + super().create_observable_counter( + name, callback, unit=unit, description=description + ) + + def create_histogram(self, name, unit="", description=""): + super().create_histogram(name, unit=unit, description=description) + + def create_observable_gauge(self, name, callback, unit="", description=""): + super().create_observable_gauge( + name, callback, unit=unit, description=description + ) + + def create_observable_up_down_counter( + self, name, callback, unit="", description="" + ): + super().create_observable_up_down_counter( + name, callback, unit=unit, description=description + ) + + +class TestMeter(TestCase): + def test_create_counter(self): + """ + Test that the meter provides a function to create a new Counter + """ + + self.assertTrue(hasattr(Meter, "create_counter")) + self.assertTrue(Meter.create_counter.__isabstractmethod__) + + def test_create_up_down_counter(self): + """ + Test that the meter provides a function to create a new UpDownCounter + """ + + self.assertTrue(hasattr(Meter, "create_up_down_counter")) + self.assertTrue(Meter.create_up_down_counter.__isabstractmethod__) + + def test_create_observable_counter(self): + """ + Test that the meter provides a function to create a new ObservableCounter + """ + + self.assertTrue(hasattr(Meter, "create_observable_counter")) + self.assertTrue(Meter.create_observable_counter.__isabstractmethod__) + + def test_create_histogram(self): + """ + Test that the meter provides a function to create a new Histogram + """ + + self.assertTrue(hasattr(Meter, "create_histogram")) + self.assertTrue(Meter.create_histogram.__isabstractmethod__) + + def test_create_observable_gauge(self): + """ + Test that the meter provides a function to create a new ObservableGauge + """ + + self.assertTrue(hasattr(Meter, "create_observable_gauge")) + self.assertTrue(Meter.create_observable_gauge.__isabstractmethod__) + + def test_create_observable_up_down_counter(self): + """ + Test that the meter provides a function to create a new + ObservableUpDownCounter + """ + + self.assertTrue(hasattr(Meter, "create_observable_up_down_counter")) + self.assertTrue( + Meter.create_observable_up_down_counter.__isabstractmethod__ + ) + + def test_no_repeated_instrument_names(self): + """ + Test that the meter returns an error when multiple instruments are + registered under the same Meter using the same name. + """ + + meter = ChildMeter("name") + + meter.create_counter("name") + + with self.assertLogs(level=WARNING): + meter.create_counter("name") + + with self.assertLogs(level=WARNING): + meter.create_up_down_counter("name") + + with self.assertLogs(level=WARNING): + meter.create_observable_counter("name", Mock()) + + with self.assertLogs(level=WARNING): + meter.create_histogram("name") + + with self.assertLogs(level=WARNING): + meter.create_observable_gauge("name", Mock()) + + with self.assertLogs(level=WARNING): + meter.create_observable_up_down_counter("name", Mock()) + + def test_same_name_instrument_different_meter(self): + """ + Test that is possible to register two instruments with the same name + under different meters. + """ + + meter_0 = ChildMeter("meter_0") + meter_1 = ChildMeter("meter_1") + + meter_0.create_counter("counter") + meter_0.create_up_down_counter("up_down_counter") + meter_0.create_observable_counter("observable_counter", Mock()) + meter_0.create_histogram("histogram") + meter_0.create_observable_gauge("observable_gauge", Mock()) + meter_0.create_observable_up_down_counter( + "observable_up_down_counter", Mock() + ) + + with self.assertRaises(AssertionError): + with self.assertLogs(level=WARNING): + meter_1.create_counter("counter") + + with self.assertRaises(AssertionError): + with self.assertLogs(level=WARNING): + meter_1.create_up_down_counter("up_down_counter") + + with self.assertRaises(AssertionError): + with self.assertLogs(level=WARNING): + meter_1.create_observable_counter("observable_counter", Mock()) + + with self.assertRaises(AssertionError): + with self.assertLogs(level=WARNING): + meter_1.create_histogram("histogram") + + with self.assertRaises(AssertionError): + with self.assertLogs(level=WARNING): + meter_1.create_observable_gauge("observable_gauge", Mock()) + + with self.assertRaises(AssertionError): + with self.assertLogs(level=WARNING): + meter_1.create_observable_up_down_counter( + "observable_up_down_counter", Mock() + ) diff --git a/opentelemetry-api/tests/metrics/test_meter_provider.py b/opentelemetry-api/tests/metrics/test_meter_provider.py new file mode 100644 index 00000000000..9784e505e03 --- /dev/null +++ b/opentelemetry-api/tests/metrics/test_meter_provider.py @@ -0,0 +1,260 @@ +# 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. +# type: ignore + +from logging import WARNING +from unittest import TestCase +from unittest.mock import Mock, patch + +from pytest import fixture + +from opentelemetry import metrics +from opentelemetry.environment_variables import OTEL_PYTHON_METER_PROVIDER +from opentelemetry.metrics import ( + ProxyMeter, + ProxyMeterProvider, + _DefaultMeter, + _DefaultMeterProvider, + get_meter_provider, + set_meter_provider, +) +from opentelemetry.metrics.instrument import ( + DefaultCounter, + DefaultHistogram, + DefaultObservableCounter, + DefaultObservableGauge, + DefaultObservableUpDownCounter, + DefaultUpDownCounter, +) + +# FIXME Test that the instrument methods can be called concurrently safely. + + +@fixture +def reset_meter_provider(): + original_meter_provider_value = metrics._METER_PROVIDER + + yield + + metrics._METER_PROVIDER = original_meter_provider_value + + +def test_set_meter_provider(reset_meter_provider): + """ + Test that the API provides a way to set a global default MeterProvider + """ + + mock = Mock() + + assert metrics._METER_PROVIDER is None + + set_meter_provider(mock) + + assert metrics._METER_PROVIDER is mock + + +def test_get_meter_provider(reset_meter_provider): + """ + Test that the API provides a way to get a global default MeterProvider + """ + + assert metrics._METER_PROVIDER is None + + assert isinstance(get_meter_provider(), ProxyMeterProvider) + + metrics._METER_PROVIDER = None + + with patch.dict( + "os.environ", {OTEL_PYTHON_METER_PROVIDER: "test_meter_provider"} + ): + + with patch("opentelemetry.metrics._load_provider", Mock()): + with patch( + "opentelemetry.metrics.cast", + Mock(**{"return_value": "test_meter_provider"}), + ): + assert get_meter_provider() == "test_meter_provider" + + +class TestGetMeter(TestCase): + def test_get_meter_parameters(self): + """ + Test that get_meter accepts name, version and schema_url + """ + try: + _DefaultMeterProvider().get_meter( + "name", version="version", schema_url="schema_url" + ) + except Exception as error: + self.fail(f"Unexpected exception raised: {error}") + + def test_invalid_name(self): + """ + Test that when an invalid name is specified a working meter + implementation is returned as a fallback. + + Test that the fallback meter name property keeps its original invalid + value. + + Test that a message is logged reporting the specified value for the + fallback meter is invalid. + """ + with self.assertLogs(level=WARNING): + meter = _DefaultMeterProvider().get_meter("") + + self.assertTrue(isinstance(meter, _DefaultMeter)) + + self.assertEqual(meter.name, "") + + with self.assertLogs(level=WARNING): + meter = _DefaultMeterProvider().get_meter(None) + + self.assertTrue(isinstance(meter, _DefaultMeter)) + + self.assertEqual(meter.name, None) + + +class MockProvider(_DefaultMeterProvider): + def get_meter(self, name, version=None, schema_url=None): + return MockMeter(name, version=version, schema_url=schema_url) + + +class MockMeter(_DefaultMeter): + def create_counter(self, name, unit="", description=""): + return MockCounter("name") + + def create_up_down_counter(self, name, unit="", description=""): + return MockUpDownCounter("name") + + def create_observable_counter( + self, name, callback, unit="", description="" + ): + return MockObservableCounter("name", callback) + + def create_histogram(self, name, unit="", description=""): + return MockHistogram("name") + + def create_observable_gauge(self, name, callback, unit="", description=""): + return MockObservableGauge("name", callback) + + def create_observable_up_down_counter( + self, name, callback, unit="", description="" + ): + return MockObservableUpDownCounter("name", callback) + + +class MockCounter(DefaultCounter): + pass + + +class MockHistogram(DefaultHistogram): + pass + + +class MockObservableCounter(DefaultObservableCounter): + pass + + +class MockObservableGauge(DefaultObservableGauge): + pass + + +class MockObservableUpDownCounter(DefaultObservableUpDownCounter): + pass + + +class MockUpDownCounter(DefaultUpDownCounter): + pass + + +class TestProxy(TestCase): + def test_proxy_meter(self): + + """ + Test that the proxy meter provider and proxy meter automatically point + to updated objects. + """ + + original_provider = metrics._METER_PROVIDER + + provider = get_meter_provider() + self.assertIsInstance(provider, ProxyMeterProvider) + + meter = provider.get_meter("proxy-test") + self.assertIsInstance(meter, ProxyMeter) + + self.assertIsInstance(meter.create_counter("counter0"), DefaultCounter) + + self.assertIsInstance( + meter.create_histogram("histogram0"), DefaultHistogram + ) + + def callback(): + yield + + self.assertIsInstance( + meter.create_observable_counter("observable_counter0", callback()), + DefaultObservableCounter, + ) + + self.assertIsInstance( + meter.create_observable_gauge("observable_gauge0", callback()), + DefaultObservableGauge, + ) + + self.assertIsInstance( + meter.create_observable_up_down_counter( + "observable_up_down_counter0", callback() + ), + DefaultObservableUpDownCounter, + ) + + self.assertIsInstance( + meter.create_up_down_counter("up_down_counter0"), + DefaultUpDownCounter, + ) + + set_meter_provider(MockProvider()) + + self.assertIsInstance(get_meter_provider(), MockProvider) + self.assertIsInstance(provider.get_meter("proxy-test"), MockMeter) + + self.assertIsInstance(meter.create_counter("counter1"), MockCounter) + + self.assertIsInstance( + meter.create_histogram("histogram1"), MockHistogram + ) + + self.assertIsInstance( + meter.create_observable_counter("observable_counter1", callback()), + MockObservableCounter, + ) + + self.assertIsInstance( + meter.create_observable_gauge("observable_gauge1", callback()), + MockObservableGauge, + ) + + self.assertIsInstance( + meter.create_observable_up_down_counter( + "observable_up_down_counter1", callback() + ), + MockObservableUpDownCounter, + ) + + self.assertIsInstance( + meter.create_up_down_counter("up_down_counter1"), MockUpDownCounter + ) + + metrics._METER_PROVIDER = original_provider diff --git a/tests/util/src/opentelemetry/test/__init__.py b/tests/util/src/opentelemetry/test/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/tox.ini b/tox.ini index 0210e7e6e9d..e78f0226837 100644 --- a/tox.ini +++ b/tox.ini @@ -107,7 +107,7 @@ changedir = exporter-zipkin-combined: exporter/opentelemetry-exporter-zipkin/tests exporter-zipkin-proto-http: exporter/opentelemetry-exporter-zipkin-proto-http/tests exporter-zipkin-json: exporter/opentelemetry-exporter-zipkin-json/tests - + propagator-b3: propagator/opentelemetry-propagator-b3/tests propagator-jaeger: propagator/opentelemetry-propagator-jaeger/tests From 84a3111138c2309558c5ecfb9545b300ae925705 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Wed, 29 Sep 2021 12:13:16 -0400 Subject: [PATCH 12/13] Make measurement a concrete class (#2153) * Make Measurement a concrete class * comments * update changelog --- CHANGELOG.md | 2 + .../src/opentelemetry/metrics/measurement.py | 41 ++++++---- .../metrics/integration_test/test_cpu_time.py | 80 +++++++++---------- .../tests/metrics/test_instruments.py | 19 ++--- .../tests/metrics/test_measurement.py | 46 +++++++++++ 5 files changed, 118 insertions(+), 70 deletions(-) create mode 100644 opentelemetry-api/tests/metrics/test_measurement.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1be15da6312..75ee2b57142 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 ([#2182](https://github.com/open-telemetry/opentelemetry-python/pull/2182)) - Automatically load OTEL environment variables as options for `opentelemetry-instrument` ([#1969](https://github.com/open-telemetry/opentelemetry-python/pull/1969)) +- Make Measurement a concrete class + ([#2153](https://github.com/open-telemetry/opentelemetry-python/pull/2153)) - Add metrics API ([#1887](https://github.com/open-telemetry/opentelemetry-python/pull/1887)) - `opentelemetry-semantic-conventions` Update to semantic conventions v1.6.1 diff --git a/opentelemetry-api/src/opentelemetry/metrics/measurement.py b/opentelemetry-api/src/opentelemetry/metrics/measurement.py index 6b5b081c266..e8fd9725bb8 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/measurement.py +++ b/opentelemetry-api/src/opentelemetry/metrics/measurement.py @@ -12,28 +12,41 @@ # See the License for the specific language governing permissions and # limitations under the License. -# pylint: disable=too-many-ancestors -# type:ignore +from typing import Union +from opentelemetry.util.types import Attributes -from abc import ABC, abstractmethod +class Measurement: + """A measurement observed in an asynchronous instrument + + Return/yield instances of this class from asynchronous instrument callbacks. + + Args: + value: The float or int measured value + attributes: The measurement's attributes + """ + + def __init__( + self, value: Union[int, float], attributes: Attributes = None + ) -> None: + self._value = value + self._attributes = attributes -class Measurement(ABC): @property - def value(self): + def value(self) -> Union[float, int]: return self._value @property - def attributes(self): + def attributes(self) -> Attributes: return self._attributes - @abstractmethod - def __init__(self, value, attributes=None): - self._value = value - self._attributes = attributes - + def __eq__(self, other: object) -> bool: + return ( + isinstance(other, Measurement) + and self.value == other.value + and self.attributes == other.attributes + ) -class DefaultMeasurement(Measurement): - def __init__(self, value, attributes=None): - super().__init__(value, attributes=attributes) + def __repr__(self) -> str: + return f"Measurement(value={self.value}, attributes={self.attributes})" diff --git a/opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py b/opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py index 347f6c4dc48..561f0c06582 100644 --- a/opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py +++ b/opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py @@ -23,14 +23,6 @@ # FIXME Test that the instrument methods can be called concurrently safely. -class ChildMeasurement(Measurement): - def __init__(self, value, attributes=None): - super().__init__(value, attributes=attributes) - - def __eq__(self, o: Measurement) -> bool: - return self.value == o.value and self.attributes == o.attributes - - class TestCpuTimeIntegration(TestCase): """Integration test of scraping CPU time from proc stat with an observable counter""" @@ -48,24 +40,24 @@ class TestCpuTimeIntegration(TestCase): softirq 1644603067 0 166540056 208 309152755 8936439 0 1354908 935642970 13 222975718\n""" measurements_expected = [ - ChildMeasurement(6150, {"cpu": "cpu0", "state": "user"}), - ChildMeasurement(3177, {"cpu": "cpu0", "state": "nice"}), - ChildMeasurement(5946, {"cpu": "cpu0", "state": "system"}), - ChildMeasurement(891264, {"cpu": "cpu0", "state": "idle"}), - ChildMeasurement(1296, {"cpu": "cpu0", "state": "iowait"}), - ChildMeasurement(0, {"cpu": "cpu0", "state": "irq"}), - ChildMeasurement(8343, {"cpu": "cpu0", "state": "softirq"}), - ChildMeasurement(421, {"cpu": "cpu0", "state": "guest"}), - ChildMeasurement(0, {"cpu": "cpu0", "state": "guest_nice"}), - ChildMeasurement(5882, {"cpu": "cpu1", "state": "user"}), - ChildMeasurement(3491, {"cpu": "cpu1", "state": "nice"}), - ChildMeasurement(6404, {"cpu": "cpu1", "state": "system"}), - ChildMeasurement(891564, {"cpu": "cpu1", "state": "idle"}), - ChildMeasurement(1244, {"cpu": "cpu1", "state": "iowait"}), - ChildMeasurement(0, {"cpu": "cpu1", "state": "irq"}), - ChildMeasurement(2410, {"cpu": "cpu1", "state": "softirq"}), - ChildMeasurement(418, {"cpu": "cpu1", "state": "guest"}), - ChildMeasurement(0, {"cpu": "cpu1", "state": "guest_nice"}), + Measurement(6150, {"cpu": "cpu0", "state": "user"}), + Measurement(3177, {"cpu": "cpu0", "state": "nice"}), + Measurement(5946, {"cpu": "cpu0", "state": "system"}), + Measurement(891264, {"cpu": "cpu0", "state": "idle"}), + Measurement(1296, {"cpu": "cpu0", "state": "iowait"}), + Measurement(0, {"cpu": "cpu0", "state": "irq"}), + Measurement(8343, {"cpu": "cpu0", "state": "softirq"}), + Measurement(421, {"cpu": "cpu0", "state": "guest"}), + Measurement(0, {"cpu": "cpu0", "state": "guest_nice"}), + Measurement(5882, {"cpu": "cpu1", "state": "user"}), + Measurement(3491, {"cpu": "cpu1", "state": "nice"}), + Measurement(6404, {"cpu": "cpu1", "state": "system"}), + Measurement(891564, {"cpu": "cpu1", "state": "idle"}), + Measurement(1244, {"cpu": "cpu1", "state": "iowait"}), + Measurement(0, {"cpu": "cpu1", "state": "irq"}), + Measurement(2410, {"cpu": "cpu1", "state": "softirq"}), + Measurement(418, {"cpu": "cpu1", "state": "guest"}), + Measurement(0, {"cpu": "cpu1", "state": "guest_nice"}), ] def test_cpu_time_callback(self): @@ -78,31 +70,31 @@ def cpu_time_callback() -> Iterable[Measurement]: if not line.startswith("cpu"): break cpu, *states = line.split() - yield ChildMeasurement( + yield Measurement( int(states[0]) // 100, {"cpu": cpu, "state": "user"} ) - yield ChildMeasurement( + yield Measurement( int(states[1]) // 100, {"cpu": cpu, "state": "nice"} ) - yield ChildMeasurement( + yield Measurement( int(states[2]) // 100, {"cpu": cpu, "state": "system"} ) - yield ChildMeasurement( + yield Measurement( int(states[3]) // 100, {"cpu": cpu, "state": "idle"} ) - yield ChildMeasurement( + yield Measurement( int(states[4]) // 100, {"cpu": cpu, "state": "iowait"} ) - yield ChildMeasurement( + yield Measurement( int(states[5]) // 100, {"cpu": cpu, "state": "irq"} ) - yield ChildMeasurement( + yield Measurement( int(states[6]) // 100, {"cpu": cpu, "state": "softirq"} ) - yield ChildMeasurement( + yield Measurement( int(states[7]) // 100, {"cpu": cpu, "state": "guest"} ) - yield ChildMeasurement( + yield Measurement( int(states[8]) // 100, {"cpu": cpu, "state": "guest_nice"} ) @@ -130,54 +122,54 @@ def cpu_time_generator() -> Generator[ break cpu, *states = line.split() measurements.append( - ChildMeasurement( + Measurement( int(states[0]) // 100, {"cpu": cpu, "state": "user"}, ) ) measurements.append( - ChildMeasurement( + Measurement( int(states[1]) // 100, {"cpu": cpu, "state": "nice"}, ) ) measurements.append( - ChildMeasurement( + Measurement( int(states[2]) // 100, {"cpu": cpu, "state": "system"}, ) ) measurements.append( - ChildMeasurement( + Measurement( int(states[3]) // 100, {"cpu": cpu, "state": "idle"}, ) ) measurements.append( - ChildMeasurement( + Measurement( int(states[4]) // 100, {"cpu": cpu, "state": "iowait"}, ) ) measurements.append( - ChildMeasurement( + Measurement( int(states[5]) // 100, {"cpu": cpu, "state": "irq"} ) ) measurements.append( - ChildMeasurement( + Measurement( int(states[6]) // 100, {"cpu": cpu, "state": "softirq"}, ) ) measurements.append( - ChildMeasurement( + Measurement( int(states[7]) // 100, {"cpu": cpu, "state": "guest"}, ) ) measurements.append( - ChildMeasurement( + Measurement( int(states[8]) // 100, {"cpu": cpu, "state": "guest_nice"}, ) diff --git a/opentelemetry-api/tests/metrics/test_instruments.py b/opentelemetry-api/tests/metrics/test_instruments.py index 2dd100c9ed7..9be227ec6f6 100644 --- a/opentelemetry-api/tests/metrics/test_instruments.py +++ b/opentelemetry-api/tests/metrics/test_instruments.py @@ -45,11 +45,6 @@ def __init__(self, name, *args, unit="", description="", **kwargs): ) -class ChildMeasurement(Measurement): - def __init__(self, value, attributes=None): - super().__init__(value, attributes=attributes) - - class TestInstrument(TestCase): def test_instrument_has_name(self): """ @@ -341,8 +336,8 @@ def callback(): list(observable_counter.callback()) def callback(): - yield [ChildMeasurement(1), ChildMeasurement(2)] - yield [ChildMeasurement(-1)] + yield [Measurement(1), Measurement(2)] + yield [Measurement(-1)] observable_counter = DefaultObservableCounter("name", callback()) @@ -382,7 +377,7 @@ def callback_invalid_return(): list(observable_counter.callback()) def callback_valid(): - return [ChildMeasurement(1), ChildMeasurement(2)] + return [Measurement(1), Measurement(2)] observable_counter = DefaultObservableCounter("name", callback_valid) @@ -391,7 +386,7 @@ def callback_valid(): list(observable_counter.callback()) def callback_one_invalid(): - return [ChildMeasurement(1), ChildMeasurement(-2)] + return [Measurement(1), Measurement(-2)] observable_counter = DefaultObservableCounter( "name", callback_one_invalid @@ -578,7 +573,7 @@ def callback(): list(observable_gauge.callback()) def callback(): - yield [ChildMeasurement(1), ChildMeasurement(-1)] + yield [Measurement(1), Measurement(-1)] observable_gauge = DefaultObservableGauge("name", callback()) with self.assertRaises(AssertionError): @@ -786,8 +781,8 @@ def test_observable_up_down_counter_callback(self): ) def callback(): - yield ChildMeasurement(1) - yield ChildMeasurement(-1) + yield Measurement(1) + yield Measurement(-1) with self.assertRaises(AssertionError): with self.assertLogs(level=ERROR): diff --git a/opentelemetry-api/tests/metrics/test_measurement.py b/opentelemetry-api/tests/metrics/test_measurement.py new file mode 100644 index 00000000000..7fcd33d04f9 --- /dev/null +++ b/opentelemetry-api/tests/metrics/test_measurement.py @@ -0,0 +1,46 @@ +# 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. + +from unittest import TestCase + +from opentelemetry.metrics.measurement import Measurement + + +class TestMeasurement(TestCase): + def test_measurement_init(self): + try: + # int + Measurement(321, {"hello": "world"}) + + # float + Measurement(321.321, {"hello": "world"}) + except Exception: # pylint: disable=broad-except + self.fail( + "Unexpected exception raised when instantiating Measurement" + ) + + def test_measurement_equality(self): + self.assertEqual( + Measurement(321, {"hello": "world"}), + Measurement(321, {"hello": "world"}), + ) + + self.assertNotEqual( + Measurement(321, {"hello": "world"}), + Measurement(321.321, {"hello": "world"}), + ) + self.assertNotEqual( + Measurement(321, {"baz": "world"}), + Measurement(321, {"hello": "world"}), + ) From 450a86c26c7e0ffa18b458e09fc50b3017c1892f Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 12 Oct 2021 16:20:07 +0200 Subject: [PATCH 13/13] Remove checks and callbacks from API (#2164) * Remove checks and callbacks from API Fixes #2151 * Fix tests --- .../src/opentelemetry/metrics/__init__.py | 24 +- .../src/opentelemetry/metrics/instrument.py | 80 +----- .../metrics/integration_test/test_cpu_time.py | 4 +- .../tests/metrics/test_instruments.py | 236 ------------------ opentelemetry-api/tests/metrics/test_meter.py | 74 ------ .../tests/metrics/test_meter_provider.py | 11 +- 6 files changed, 25 insertions(+), 404 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index 83d210e063b..2a60f0e2029 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -55,8 +55,7 @@ def get_meter( version=None, schema_url=None, ) -> "Meter": - if name is None or name == "": - _logger.warning("Invalid name: %s", name) + pass class _DefaultMeterProvider(MeterProvider): @@ -104,24 +103,17 @@ def version(self): def schema_url(self): return self._schema_url - def _secure_instrument_name(self, name): - name = name.lower() - - if name in self._instrument_names: - _logger.error("Instrument name %s has been used already", name) - - else: - self._instrument_names.add(name) + # FIXME check that the instrument name has not been used already @abstractmethod def create_counter(self, name, unit="", description="") -> Counter: - self._secure_instrument_name(name) + pass @abstractmethod def create_up_down_counter( self, name, unit="", description="" ) -> UpDownCounter: - self._secure_instrument_name(name) + pass @abstractmethod def create_observable_counter( @@ -206,23 +198,21 @@ def cpu_time_callback(states_to_include: set[str]) -> Iterable[Iterable[Measurem description: A description for this instrument and what it measures. """ - self._secure_instrument_name(name) - @abstractmethod def create_histogram(self, name, unit="", description="") -> Histogram: - self._secure_instrument_name(name) + pass @abstractmethod def create_observable_gauge( self, name, callback, unit="", description="" ) -> ObservableGauge: - self._secure_instrument_name(name) + pass @abstractmethod def create_observable_up_down_counter( self, name, callback, unit="", description="" ) -> ObservableUpDownCounter: - self._secure_instrument_name(name) + pass class ProxyMeter(Meter): diff --git a/opentelemetry-api/src/opentelemetry/metrics/instrument.py b/opentelemetry-api/src/opentelemetry/metrics/instrument.py index 5d382056408..63e9dddf91c 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/instrument.py +++ b/opentelemetry-api/src/opentelemetry/metrics/instrument.py @@ -19,7 +19,6 @@ from abc import ABC, abstractmethod from collections import abc as collections_abc from logging import getLogger -from re import compile as compile_ from typing import Callable, Generator, Iterable, Union from opentelemetry.metrics.measurement import Measurement @@ -33,44 +32,13 @@ class Instrument(ABC): - - _name_regex = compile_(r"[a-zA-Z][-.\w]{0,62}") - - @property - def name(self): - return self._name - - @property - def unit(self): - return self._unit - - @property - def description(self): - return self._description - @abstractmethod def __init__(self, name, unit="", description=""): + pass - if name is None or self._name_regex.fullmatch(name) is None: - _logger.error("Invalid instrument name %s", name) - - else: - self._name = name - - if unit is None: - self._unit = "" - elif len(unit) > 63: - _logger.error("unit must be 63 characters or shorter") - - elif any(ord(character) > 127 for character in unit): - _logger.error("unit must only contain ASCII characters") - else: - self._unit = unit - - if description is None: - description = "" - - self._description = description + # FIXME check that the instrument name is valid + # FIXME check that the unit is 63 characters or shorter + # FIXME check that the unit contains only ASCII characters class Synchronous(Instrument): @@ -96,11 +64,10 @@ def __init__( self._callback = callback elif isinstance(callback, collections_abc.Generator): self._callback = self._wrap_generator_callback(callback) - else: - _logger.error("callback must be a callable or generator") + # FIXME check that callback is a callable or generator + @staticmethod def _wrap_generator_callback( - self, generator_callback: _TInstrumentCallbackGenerator, ) -> _TInstrumentCallback: """Wraps a generator style callback into a callable one""" @@ -115,30 +82,13 @@ def inner() -> Iterable[Measurement]: return next(generator_callback) except StopIteration: has_items = False - _logger.error( - "callback generator for instrument %s ran out of measurements", - self._name, - ) + # FIXME handle the situation where the callback generator has + # run out of measurements return [] return inner - def callback(self): - measurements = self._callback() - if not isinstance(measurements, collections_abc.Iterable): - _logger.error( - "Callback must return an iterable of Measurement, got %s", - type(measurements), - ) - return - for measurement in measurements: - if not isinstance(measurement, Measurement): - _logger.error( - "Callback must return an iterable of Measurement, " - "iterable contained type %s", - type(measurement), - ) - yield measurement + # FIXME check that callbacks return an iterable of Measurements class _Adding(Instrument): @@ -160,8 +110,8 @@ class _NonMonotonic(_Adding): class Counter(_Monotonic, Synchronous): @abstractmethod def add(self, amount, attributes=None): - if amount < 0: - _logger.error("Amount must be non-negative") + # FIXME check that the amount is non negative + pass class DefaultCounter(Counter): @@ -187,13 +137,7 @@ def add(self, amount, attributes=None): class ObservableCounter(_Monotonic, Asynchronous): - def callback(self): - measurements = super().callback() - - for measurement in measurements: - if measurement.value < 0: - _logger.error("Amount must be non-negative") - yield measurement + pass class DefaultObservableCounter(ObservableCounter): diff --git a/opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py b/opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py index 561f0c06582..a45b6dfc503 100644 --- a/opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py +++ b/opentelemetry-api/tests/metrics/integration_test/test_cpu_time.py @@ -104,7 +104,7 @@ def cpu_time_callback() -> Iterable[Measurement]: unit="s", description="CPU time", ) - measurements = list(observable_counter.callback()) + measurements = list(observable_counter._callback()) self.assertEqual(measurements, self.measurements_expected) def test_cpu_time_generator(self): @@ -182,5 +182,5 @@ def cpu_time_generator() -> Generator[ unit="s", description="CPU time", ) - measurements = list(observable_counter.callback()) + measurements = list(observable_counter._callback()) self.assertEqual(measurements, self.measurements_expected) diff --git a/opentelemetry-api/tests/metrics/test_instruments.py b/opentelemetry-api/tests/metrics/test_instruments.py index 9be227ec6f6..05adaa43988 100644 --- a/opentelemetry-api/tests/metrics/test_instruments.py +++ b/opentelemetry-api/tests/metrics/test_instruments.py @@ -14,7 +14,6 @@ # type: ignore from inspect import Signature, isabstract, signature -from logging import ERROR from unittest import TestCase from opentelemetry.metrics import Meter, _DefaultMeter @@ -22,9 +21,6 @@ Counter, DefaultCounter, DefaultHistogram, - DefaultObservableCounter, - DefaultObservableGauge, - DefaultObservableUpDownCounter, DefaultUpDownCounter, Histogram, Instrument, @@ -33,7 +29,6 @@ ObservableUpDownCounter, UpDownCounter, ) -from opentelemetry.metrics.measurement import Measurement # FIXME Test that the instrument methods can be called concurrently safely. @@ -45,117 +40,6 @@ def __init__(self, name, *args, unit="", description="", **kwargs): ) -class TestInstrument(TestCase): - def test_instrument_has_name(self): - """ - Test that the instrument has name. - """ - - init_signature = signature(Instrument.__init__) - self.assertIn("name", init_signature.parameters.keys()) - self.assertIs( - init_signature.parameters["name"].default, Signature.empty - ) - - self.assertTrue(hasattr(Instrument, "name")) - - def test_instrument_has_unit(self): - """ - Test that the instrument has unit. - """ - - init_signature = signature(Instrument.__init__) - self.assertIn("unit", init_signature.parameters.keys()) - self.assertIs(init_signature.parameters["unit"].default, "") - - self.assertTrue(hasattr(Instrument, "unit")) - - def test_instrument_has_description(self): - """ - Test that the instrument has description. - """ - - init_signature = signature(Instrument.__init__) - self.assertIn("description", init_signature.parameters.keys()) - self.assertIs(init_signature.parameters["description"].default, "") - - self.assertTrue(hasattr(Instrument, "description")) - - def test_instrument_name_syntax(self): - """ - Test that instrument names conform to the specified syntax. - """ - - with self.assertLogs(level=ERROR): - ChildInstrument("") - - with self.assertLogs(level=ERROR): - ChildInstrument(None) - - with self.assertLogs(level=ERROR): - ChildInstrument("1a") - - with self.assertLogs(level=ERROR): - ChildInstrument("_a") - - with self.assertLogs(level=ERROR): - ChildInstrument("!a ") - - with self.assertLogs(level=ERROR): - ChildInstrument("a ") - - with self.assertLogs(level=ERROR): - ChildInstrument("a%") - - with self.assertLogs(level=ERROR): - ChildInstrument("a" * 64) - - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - ChildInstrument("abc_def_ghi") - - def test_instrument_unit_syntax(self): - """ - Test that instrument unit conform to the specified syntax. - """ - - with self.assertLogs(level=ERROR): - ChildInstrument("name", unit="a" * 64) - - with self.assertLogs(level=ERROR): - ChildInstrument("name", unit="ñ") - - child_instrument = ChildInstrument("name", unit="a") - self.assertEqual(child_instrument.unit, "a") - - child_instrument = ChildInstrument("name", unit="A") - self.assertEqual(child_instrument.unit, "A") - - child_instrument = ChildInstrument("name") - self.assertEqual(child_instrument.unit, "") - - child_instrument = ChildInstrument("name", unit=None) - self.assertEqual(child_instrument.unit, "") - - def test_instrument_description_syntax(self): - """ - Test that instrument description conform to the specified syntax. - """ - - child_instrument = ChildInstrument("name", description="a") - self.assertEqual(child_instrument.description, "a") - - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - ChildInstrument("name", description="a" * 1024) - - child_instrument = ChildInstrument("name") - self.assertEqual(child_instrument.description, "") - - child_instrument = ChildInstrument("name", description=None) - self.assertEqual(child_instrument.description, "") - - class TestCounter(TestCase): def test_create_counter(self): """ @@ -221,9 +105,6 @@ def test_counter_add_method(self): add_signature.parameters["amount"].default, Signature.empty ) - with self.assertLogs(level=ERROR): - DefaultCounter("name").add(-1) - class TestObservableCounter(TestCase): def test_create_observable_counter(self): @@ -322,79 +203,6 @@ def test_observable_counter_generator(self): Signature.empty, ) - def callback(): - yield 1 - - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - observable_counter = DefaultObservableCounter( - "name", callback() - ) - - with self.assertLogs(level=ERROR): - # use list() to consume the whole generator returned by callback() - list(observable_counter.callback()) - - def callback(): - yield [Measurement(1), Measurement(2)] - yield [Measurement(-1)] - - observable_counter = DefaultObservableCounter("name", callback()) - - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - list(observable_counter.callback()) - - with self.assertLogs(level=ERROR): - list(observable_counter.callback()) - - # out of items in generator, should log once - with self.assertLogs(level=ERROR): - list(observable_counter.callback()) - - # but log only once - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - list(observable_counter.callback()) - - def test_observable_counter_callback(self): - """ - Equivalent to test_observable_counter_generator but uses the callback - form. - """ - - def callback_invalid_return(): - return 1 - - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - observable_counter = DefaultObservableCounter( - "name", callback_invalid_return - ) - - with self.assertLogs(level=ERROR): - # use list() to consume the whole generator returned by callback() - list(observable_counter.callback()) - - def callback_valid(): - return [Measurement(1), Measurement(2)] - - observable_counter = DefaultObservableCounter("name", callback_valid) - - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - list(observable_counter.callback()) - - def callback_one_invalid(): - return [Measurement(1), Measurement(-2)] - - observable_counter = DefaultObservableCounter( - "name", callback_one_invalid - ) - - with self.assertLogs(level=ERROR): - list(observable_counter.callback()) - class TestHistogram(TestCase): def test_create_histogram(self): @@ -562,24 +370,6 @@ def test_observable_gauge_callback(self): Signature.empty, ) - def callback(): - yield - - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - observable_gauge = DefaultObservableGauge("name", callback()) - - with self.assertLogs(level=ERROR): - list(observable_gauge.callback()) - - def callback(): - yield [Measurement(1), Measurement(-1)] - - observable_gauge = DefaultObservableGauge("name", callback()) - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - list(observable_gauge.callback()) - class TestUpDownCounter(TestCase): def test_create_up_down_counter(self): @@ -662,14 +452,6 @@ def test_up_down_counter_add_method(self): add_signature.parameters["amount"].default, Signature.empty ) - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - DefaultUpDownCounter("name").add(-1) - - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - DefaultUpDownCounter("name").add(1) - class TestObservableUpDownCounter(TestCase): def test_create_observable_up_down_counter(self): @@ -779,21 +561,3 @@ def test_observable_up_down_counter_callback(self): ].default, Signature.empty, ) - - def callback(): - yield Measurement(1) - yield Measurement(-1) - - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - observable_up_down_counter = DefaultObservableUpDownCounter( - "name", callback() - ) - - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - observable_up_down_counter.callback() - - with self.assertRaises(AssertionError): - with self.assertLogs(level=ERROR): - observable_up_down_counter.callback() diff --git a/opentelemetry-api/tests/metrics/test_meter.py b/opentelemetry-api/tests/metrics/test_meter.py index 96543b69fe2..5c39526ca62 100644 --- a/opentelemetry-api/tests/metrics/test_meter.py +++ b/opentelemetry-api/tests/metrics/test_meter.py @@ -13,9 +13,7 @@ # limitations under the License. # type: ignore -from logging import WARNING from unittest import TestCase -from unittest.mock import Mock from opentelemetry.metrics import Meter @@ -105,75 +103,3 @@ def test_create_observable_up_down_counter(self): self.assertTrue( Meter.create_observable_up_down_counter.__isabstractmethod__ ) - - def test_no_repeated_instrument_names(self): - """ - Test that the meter returns an error when multiple instruments are - registered under the same Meter using the same name. - """ - - meter = ChildMeter("name") - - meter.create_counter("name") - - with self.assertLogs(level=WARNING): - meter.create_counter("name") - - with self.assertLogs(level=WARNING): - meter.create_up_down_counter("name") - - with self.assertLogs(level=WARNING): - meter.create_observable_counter("name", Mock()) - - with self.assertLogs(level=WARNING): - meter.create_histogram("name") - - with self.assertLogs(level=WARNING): - meter.create_observable_gauge("name", Mock()) - - with self.assertLogs(level=WARNING): - meter.create_observable_up_down_counter("name", Mock()) - - def test_same_name_instrument_different_meter(self): - """ - Test that is possible to register two instruments with the same name - under different meters. - """ - - meter_0 = ChildMeter("meter_0") - meter_1 = ChildMeter("meter_1") - - meter_0.create_counter("counter") - meter_0.create_up_down_counter("up_down_counter") - meter_0.create_observable_counter("observable_counter", Mock()) - meter_0.create_histogram("histogram") - meter_0.create_observable_gauge("observable_gauge", Mock()) - meter_0.create_observable_up_down_counter( - "observable_up_down_counter", Mock() - ) - - with self.assertRaises(AssertionError): - with self.assertLogs(level=WARNING): - meter_1.create_counter("counter") - - with self.assertRaises(AssertionError): - with self.assertLogs(level=WARNING): - meter_1.create_up_down_counter("up_down_counter") - - with self.assertRaises(AssertionError): - with self.assertLogs(level=WARNING): - meter_1.create_observable_counter("observable_counter", Mock()) - - with self.assertRaises(AssertionError): - with self.assertLogs(level=WARNING): - meter_1.create_histogram("histogram") - - with self.assertRaises(AssertionError): - with self.assertLogs(level=WARNING): - meter_1.create_observable_gauge("observable_gauge", Mock()) - - with self.assertRaises(AssertionError): - with self.assertLogs(level=WARNING): - meter_1.create_observable_up_down_counter( - "observable_up_down_counter", Mock() - ) diff --git a/opentelemetry-api/tests/metrics/test_meter_provider.py b/opentelemetry-api/tests/metrics/test_meter_provider.py index 9784e505e03..c78de94cc7a 100644 --- a/opentelemetry-api/tests/metrics/test_meter_provider.py +++ b/opentelemetry-api/tests/metrics/test_meter_provider.py @@ -13,7 +13,6 @@ # limitations under the License. # type: ignore -from logging import WARNING from unittest import TestCase from unittest.mock import Mock, patch @@ -110,17 +109,15 @@ def test_invalid_name(self): Test that a message is logged reporting the specified value for the fallback meter is invalid. """ - with self.assertLogs(level=WARNING): - meter = _DefaultMeterProvider().get_meter("") + meter = _DefaultMeterProvider().get_meter("") - self.assertTrue(isinstance(meter, _DefaultMeter)) + self.assertTrue(isinstance(meter, _DefaultMeter)) self.assertEqual(meter.name, "") - with self.assertLogs(level=WARNING): - meter = _DefaultMeterProvider().get_meter(None) + meter = _DefaultMeterProvider().get_meter(None) - self.assertTrue(isinstance(meter, _DefaultMeter)) + self.assertTrue(isinstance(meter, _DefaultMeter)) self.assertEqual(meter.name, None)