From 82993a3fb4ae08b880240aa32c7a288c71ace1c1 Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Mon, 5 Feb 2024 14:27:38 +0100 Subject: [PATCH 01/15] changed the default otel-LogHandler to format the original logrecord before emitting it as the body, and retain all meaningful attributes of the original logrecord. --- README.md | 3 +- .../sdk/_logs/_internal/__init__.py | 85 +++---------------- opentelemetry-sdk/tests/logs/test_export.py | 4 +- opentelemetry-sdk/tests/logs/test_handler.py | 78 ++++++++++++++++- 4 files changed, 90 insertions(+), 80 deletions(-) diff --git a/README.md b/README.md index c36a64062c9..e46622adbe7 100644 --- a/README.md +++ b/README.md @@ -80,8 +80,7 @@ this repository and perform an [editable install](https://pip.pypa.io/en/stable/reference/pip_install/#editable-installs): ```sh -pip install -e ./opentelemetry-api -pip install -e ./opentelemetry-sdk +pip install -e ./opentelemetry-sdk/ -e ./opentelemetry-api/ -e ./opentelemetry-semantic-conventions/ pip install -e ./instrumentation/opentelemetry-instrumentation-{instrumentation} ``` diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index cfa4d6cfa9b..c6e37d4cf5a 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -402,37 +402,6 @@ def force_flush(self, timeout_millis: int = 30000) -> bool: return True -# skip natural LogRecord attributes -# http://docs.python.org/library/logging.html#logrecord-attributes -_RESERVED_ATTRS = frozenset( - ( - "asctime", - "args", - "created", - "exc_info", - "exc_text", - "filename", - "funcName", - "message", - "levelname", - "levelno", - "lineno", - "module", - "msecs", - "msg", - "name", - "pathname", - "process", - "processName", - "relativeCreated", - "stack_info", - "thread", - "threadName", - "taskName", - ) -) - - class LoggingHandler(logging.Handler): """A handler class which writes logging records, in OTLP format, to a network destination or file. Supports signals from the `logging` module. @@ -452,8 +421,17 @@ def __init__( @staticmethod def _get_attributes(record: logging.LogRecord) -> Attributes: + private_record_attrs = ( + "args", + "msg", + "stack_info", + "exc_info", + "exc_text", + ) attributes = { - k: v for k, v in vars(record).items() if k not in _RESERVED_ATTRS + k: v + for k, v in vars(record).items() + if k not in private_record_attrs } if record.exc_info: exc_type = "" @@ -478,48 +456,9 @@ def _translate(self, record: logging.LogRecord) -> LogRecord: timestamp = int(record.created * 1e9) span_context = get_current_span().get_span_context() attributes = self._get_attributes(record) - # This comment is taken from GanyedeNil's PR #3343, I have redacted it - # slightly for clarity: - # According to the definition of the Body field type in the - # OTel 1.22.0 Logs Data Model article, the Body field should be of - # type 'any' and should not use the str method to directly translate - # the msg. This is because str only converts non-text types into a - # human-readable form, rather than a standard format, which leads to - # the need for additional operations when collected through a log - # collector. - # Considering that he Body field should be of type 'any' and should not - # use the str method but record.msg is also a string type, then the - # difference is just the self.args formatting? - # The primary consideration depends on the ultimate purpose of the log. - # Converting the default log directly into a string is acceptable as it - # will be required to be presented in a more readable format. However, - # this approach might not be as "standard" when hoping to aggregate - # logs and perform subsequent data analysis. In the context of log - # extraction, it would be more appropriate for the msg to be - # converted into JSON format or remain unchanged, as it will eventually - # be transformed into JSON. If the final output JSON data contains a - # structure that appears similar to JSON but is not, it may confuse - # users. This is particularly true for operation and maintenance - # personnel who need to deal with log data in various languages. - # Where is the JSON converting occur? and what about when the msg - # represents something else but JSON, the expected behavior change? - # For the ConsoleLogExporter, it performs the to_json operation in - # opentelemetry.sdk._logs._internal.export.ConsoleLogExporter.__init__, - # so it can handle any type of input without problems. As for the - # OTLPLogExporter, it also handles any type of input encoding in - # _encode_log located in - # opentelemetry.exporter.otlp.proto.common._internal._log_encoder. - # Therefore, no extra operation is needed to support this change. - # The only thing to consider is the users who have already been using - # this SDK. If they upgrade the SDK after this change, they will need - # to readjust their logging collection rules to adapt to the latest - # output format. Therefore, this change is considered a breaking - # change and needs to be upgraded at an appropriate time. severity_number = std_to_otel(record.levelno) - if isinstance(record.msg, str) and record.args: - body = record.msg % record.args - else: - body = record.msg + body = self.format(record) + return LogRecord( timestamp=timestamp, trace_id=span_context.trace_id, diff --git a/opentelemetry-sdk/tests/logs/test_export.py b/opentelemetry-sdk/tests/logs/test_export.py index 2828504eaa9..d6727f4b43c 100644 --- a/opentelemetry-sdk/tests/logs/test_export.py +++ b/opentelemetry-sdk/tests/logs/test_export.py @@ -198,8 +198,8 @@ def test_simple_log_record_processor_different_msg_types(self): "Temperature hits high 420 C in Hyderabad", "CRITICAL", ), - (["list", "of", "strings"], "WARNING"), - ({"key": "value"}, "ERROR"), + ("['list', 'of', 'strings']", "WARNING"), + ("{'key': 'value'}", "ERROR"), ] emitted = [ (item.log_record.body, item.log_record.severity_text) diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index e126cac1720..783d4472df3 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -24,9 +24,11 @@ from opentelemetry.trace import INVALID_SPAN_CONTEXT -def get_logger(level=logging.NOTSET, logger_provider=None): +def get_logger(level=logging.NOTSET, logger_provider=None, formatter=None): logger = logging.getLogger(__name__) handler = LoggingHandler(level=level, logger_provider=logger_provider) + if formatter: + handler.setFormatter(formatter) logger.addHandler(handler) return logger @@ -112,7 +114,10 @@ def test_log_record_user_attributes(self): log_record = args[0] self.assertIsNotNone(log_record) - self.assertEqual(log_record.attributes, {"http.status_code": 200}) + self.assertEqual( + log_record.attributes, + {**log_record.attributes, **{"http.status_code": 200}}, + ) self.assertTrue(isinstance(log_record.attributes, BoundedAttributes)) def test_log_record_exception(self): @@ -131,7 +136,7 @@ def test_log_record_exception(self): log_record = args[0] self.assertIsNotNone(log_record) - self.assertEqual(log_record.body, "Zero Division Error") + self.assertIn("Zero Division Error", log_record.body) self.assertEqual( log_record.attributes[SpanAttributes.EXCEPTION_TYPE], ZeroDivisionError.__name__, @@ -195,3 +200,70 @@ def test_log_record_trace_correlation(self): self.assertEqual(log_record.trace_id, span_context.trace_id) self.assertEqual(log_record.span_id, span_context.span_id) self.assertEqual(log_record.trace_flags, span_context.trace_flags) + + def test_original_record_args_are_retained(self): + emitter_provider_mock = Mock(spec=LoggerProvider) + emitter_mock = APIGetLogger( + __name__, logger_provider=emitter_provider_mock + ) + logger = get_logger(logger_provider=emitter_provider_mock) + + with self.assertLogs(level=logging.INFO): + logger.info("Test message") + args, _ = emitter_mock.emit.call_args_list[0] + log_record = args[0] + + self.assertEqual( + set(log_record.attributes), + { + "message", + "created", + "filename", + "funcName", + "levelname", + "levelno", + "lineno", + "module", + "msecs", + "name", + "pathname", + "process", + "processName", + "relativeCreated", + "thread", + "threadName", + }, + ) + + def test_format_is_called(self): + emitter_provider_mock = Mock(spec=LoggerProvider) + emitter_mock = APIGetLogger( + __name__, logger_provider=emitter_provider_mock + ) + formatter = logging.Formatter("%(name)s - %(levelname)s - %(message)s") + logger = get_logger( + logger_provider=emitter_provider_mock, formatter=formatter + ) + + with self.assertLogs(level=logging.INFO): + logger.info("Test message") + args, _ = emitter_mock.emit.call_args_list[0] + log_record = args[0] + + self.assertEqual( + log_record.body, "tests.logs.test_handler - INFO - Test message" + ) + + def test_log_body_is_always_string(self): + emitter_provider_mock = Mock(spec=LoggerProvider) + emitter_mock = APIGetLogger( + __name__, logger_provider=emitter_provider_mock + ) + logger = get_logger(logger_provider=emitter_provider_mock) + + with self.assertLogs(level=logging.INFO): + logger.info(["something", "of", "note"]) + args, _ = emitter_mock.emit.call_args_list[0] + log_record = args[0] + + self.assertIsInstance(log_record.body, str) From b6ed8bfe20ebc44b5cadc06796cdf26306f78ac7 Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Mon, 5 Feb 2024 16:51:11 +0100 Subject: [PATCH 02/15] added a straight forward way to exclude LogRecord attributes from being retained --- .../sdk/_logs/_internal/__init__.py | 24 +++++++++++-------- opentelemetry-sdk/tests/logs/test_handler.py | 1 - 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index c6e37d4cf5a..3c927ad9bc8 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -54,6 +54,14 @@ _DEFAULT_OTEL_ATTRIBUTE_COUNT_LIMIT = 128 _ENV_VALUE_UNSET = "" +_PRIVATE_RECORD_ATTRS = { + "args", + "msg", + "message", + "stack_info", + "exc_info", + "exc_text", +} class LogLimits: @@ -412,26 +420,22 @@ def __init__( self, level=logging.NOTSET, logger_provider=None, + exclude_attributes=None, ) -> None: super().__init__(level=level) self._logger_provider = logger_provider or get_logger_provider() self._logger = get_logger( __name__, logger_provider=self._logger_provider ) + if exclude_attributes is None: + exclude_attributes = set() + self.exclude_attributes = _PRIVATE_RECORD_ATTRS | exclude_attributes - @staticmethod - def _get_attributes(record: logging.LogRecord) -> Attributes: - private_record_attrs = ( - "args", - "msg", - "stack_info", - "exc_info", - "exc_text", - ) + def _get_attributes(self, record: logging.LogRecord) -> Attributes: attributes = { k: v for k, v in vars(record).items() - if k not in private_record_attrs + if k not in self.exclude_attributes } if record.exc_info: exc_type = "" diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 783d4472df3..c9254817e4a 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -216,7 +216,6 @@ def test_original_record_args_are_retained(self): self.assertEqual( set(log_record.attributes), { - "message", "created", "filename", "funcName", From de1d49b8b91fe662f435dbf1f79e0609f8a87efc Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Mon, 5 Feb 2024 23:26:50 +0100 Subject: [PATCH 03/15] respect semantic conventions for log record attributes --- .../sdk/_logs/_internal/__init__.py | 29 ++++++++++++------- opentelemetry-sdk/tests/logs/test_handler.py | 12 ++++---- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index 3c927ad9bc8..423eaa185cf 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -62,6 +62,13 @@ "exc_info", "exc_text", } +_SEMANTIC_CONVENTION_ATTRS = { + "pathname", + "funcName", + "lineno", + "thread", + "threadName", +} class LogLimits: @@ -416,26 +423,21 @@ class LoggingHandler(logging.Handler): https://docs.python.org/3/library/logging.html """ - def __init__( - self, - level=logging.NOTSET, - logger_provider=None, - exclude_attributes=None, - ) -> None: + def __init__(self, level=logging.NOTSET, logger_provider=None) -> None: super().__init__(level=level) self._logger_provider = logger_provider or get_logger_provider() self._logger = get_logger( __name__, logger_provider=self._logger_provider ) - if exclude_attributes is None: - exclude_attributes = set() - self.exclude_attributes = _PRIVATE_RECORD_ATTRS | exclude_attributes + self._exclude_attributes = ( + _PRIVATE_RECORD_ATTRS | _SEMANTIC_CONVENTION_ATTRS + ) def _get_attributes(self, record: logging.LogRecord) -> Attributes: attributes = { k: v for k, v in vars(record).items() - if k not in self.exclude_attributes + if k not in self._exclude_attributes } if record.exc_info: exc_type = "" @@ -454,6 +456,13 @@ def _get_attributes(self, record: logging.LogRecord) -> Attributes: attributes[SpanAttributes.EXCEPTION_TYPE] = exc_type attributes[SpanAttributes.EXCEPTION_MESSAGE] = message attributes[SpanAttributes.EXCEPTION_STACKTRACE] = stack_trace + + # adding these attributes with their semantic convention names + attributes[SpanAttributes.CODE_FILEPATH] = record.pathname + attributes[SpanAttributes.CODE_FUNCTION] = record.funcName + attributes[SpanAttributes.CODE_LINENO] = record.lineno + attributes[SpanAttributes.THREAD_ID] = record.thread + attributes[SpanAttributes.THREAD_NAME] = record.threadName return attributes def _translate(self, record: logging.LogRecord) -> LogRecord: diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index c9254817e4a..f78a8f21d38 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -201,7 +201,7 @@ def test_log_record_trace_correlation(self): self.assertEqual(log_record.span_id, span_context.span_id) self.assertEqual(log_record.trace_flags, span_context.trace_flags) - def test_original_record_args_are_retained(self): + def test_log_record_args_are_translated(self): emitter_provider_mock = Mock(spec=LoggerProvider) emitter_mock = APIGetLogger( __name__, logger_provider=emitter_provider_mock @@ -216,21 +216,21 @@ def test_original_record_args_are_retained(self): self.assertEqual( set(log_record.attributes), { + "code.filepath", + "code.function", + "code.lineno", "created", "filename", - "funcName", "levelname", "levelno", - "lineno", "module", "msecs", "name", - "pathname", "process", "processName", "relativeCreated", - "thread", - "threadName", + "thread.id", + "thread.name", }, ) From b0a5a8d287d8549c1214155a4beefc994b388998 Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Tue, 13 Feb 2024 11:08:07 +0100 Subject: [PATCH 04/15] added a comment exmplaining the reason behind log record attribute exclusion --- .../src/opentelemetry/sdk/_logs/_internal/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index ee0b5be5890..22cf273c6d7 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -55,12 +55,15 @@ _DEFAULT_OTEL_ATTRIBUTE_COUNT_LIMIT = 128 _ENV_VALUE_UNSET = "" _EXCLUDED_ATTRIBUTES = { + # pseudo-private log-record attributes, they just get dropped "args", "msg", "message", "stack_info", "exc_info", "exc_text", + # attributes that are retained, but with a different name + # following semantic conventions "pathname", "funcName", "lineno", From c9b48cffbc93472d226f1835314a503ae6ea506e Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Fri, 16 Feb 2024 11:21:12 +0100 Subject: [PATCH 05/15] exclude invalid attributes, which might also trigger endless logging loops --- .../src/opentelemetry/sdk/_logs/_internal/__init__.py | 2 +- opentelemetry-sdk/tests/logs/test_handler.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index 22cf273c6d7..0e85b464c33 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -440,7 +440,7 @@ def _get_attributes(self, record: logging.LogRecord) -> Attributes: attributes = { k: v for k, v in vars(record).items() - if k not in _EXCLUDED_ATTRIBUTES + if k not in _EXCLUDED_ATTRIBUTES and v is not None } # Add standard code attributes for logs. diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 079aae2b466..2bd82efd464 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -114,7 +114,6 @@ def test_log_record_user_attributes(self): log_record = args[0] self.assertIsNotNone(log_record) - self.assertEqual(len(log_record.attributes), 4) self.assertEqual( log_record.attributes, {**log_record.attributes, **{"http.status_code": 200}}, From a01977269777893316ea132b981c73946b49540e Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Fri, 16 Feb 2024 12:33:39 +0100 Subject: [PATCH 06/15] Merge remote-tracking branch 'origin/main' --- .github/workflows/benchmarks.yml | 2 +- .github/workflows/public-api-check.yml | 2 +- .github/workflows/test.yml | 27 +++++---- .pylintrc | 8 +-- CHANGELOG.md | 8 +++ CONTRIBUTING.md | 6 +- dev-requirements.txt | 2 +- .../flask-gunicorn/requirements.txt | 2 +- .../flask-uwsgi/requirements.txt | 2 +- docs/getting_started/tests/requirements.txt | 4 +- .../pyproject.toml | 1 - .../otlp/proto/common/_internal/__init__.py | 60 ++++++++++++++----- .../_internal/metrics_encoder/__init__.py | 35 ++++++++--- .../tests/test_backoff.py | 46 ++++++++++++++ .../pyproject.toml | 1 - .../proto/grpc/metric_exporter/__init__.py | 4 +- .../tests/test_otlp_metrics_exporter.py | 18 ++++++ .../tests/test_otlp_trace_exporter.py | 33 +++------- .../pyproject.toml | 1 - .../proto/http/metric_exporter/__init__.py | 4 +- .../metrics/test_otlp_metrics_exporter.py | 34 +++++++---- .../tests/test_proto_log_exporter.py | 18 ++---- .../tests/test_proto_span_exporter.py | 21 ++----- .../src/opentelemetry/attributes/__init__.py | 5 +- .../src/opentelemetry/trace/span.py | 10 ++-- .../tests/attributes/test_attributes.py | 17 +++--- .../sdk/_logs/_internal/__init__.py | 7 ++- .../src/opentelemetry/sdk/trace/__init__.py | 4 +- .../src/opentelemetry/sdk/util/__init__.py | 6 +- opentelemetry-sdk/tests/logs/test_handler.py | 27 +++++++++ tox.ini | 27 +++++---- 31 files changed, 283 insertions(+), 159 deletions(-) create mode 100644 exporter/opentelemetry-exporter-otlp-proto-common/tests/test_backoff.py diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index 1036cb75400..4fccfe73524 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -25,7 +25,7 @@ jobs: python-version: ${{ env[matrix.python-version] }} architecture: 'x64' - name: Install tox - run: pip install tox==3.27.1 -U tox-factor + run: pip install tox - name: Cache tox environment # Preserves .tox directory between runs for faster installs uses: actions/cache@v2 diff --git a/.github/workflows/public-api-check.yml b/.github/workflows/public-api-check.yml index 46432af0b54..67dcb798310 100644 --- a/.github/workflows/public-api-check.yml +++ b/.github/workflows/public-api-check.yml @@ -34,7 +34,7 @@ jobs: python-version: '3.10' - name: Install tox - run: pip install tox==3.27.1 -U tox-factor + run: pip install tox - name: Public API Check run: tox -e public-symbols-check diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ca1d0fa58fd..0457d584f2f 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: 2977f143df1d474735e8bdfecd91d92d534e80dc + CONTRIB_REPO_SHA: 1a984d3ba18d4080c58485b7d807dba241179d41 # This is needed because we do not clone the core repo in contrib builds anymore. # When running contrib builds as part of core builds, we use actions/checkout@v2 which # does not set an environment variable (simply just runs tox), which is different when @@ -42,9 +42,6 @@ jobs: - "getting-started" - "opentracing-shim" - "opencensus-shim" - - "exporter-jaeger-combined" - - "exporter-jaeger-proto-grpc" - - "exporter-jaeger-thrift" - "exporter-opencensus" - "exporter-otlp-proto-common" - "exporter-otlp-combined" @@ -58,6 +55,16 @@ jobs: - "propagator-b3" - "propagator-jaeger" os: [ubuntu-20.04, windows-2019] + exclude: + - python-version: pypy3 + package: "opencensus-shim" + - python-version: pypy3 + package: "exporter-opencensus" + - python-version: pypy3 + package: "exporter-otlp-combined" + - python-version: pypy3 + package: "exporter-otlp-proto-grpc" + steps: - name: Checkout Core Repo @ SHA - ${{ github.sha }} uses: actions/checkout@v2 @@ -67,7 +74,7 @@ jobs: python-version: ${{ env[matrix.python-version] }} architecture: 'x64' - name: Install tox - run: pip install tox==3.27.1 -U tox-factor + run: pip install tox - name: Cache tox environment # Preserves .tox directory between runs for faster installs uses: actions/cache@v2 @@ -75,7 +82,7 @@ jobs: path: | .tox ~/.cache/pip - key: v3-tox-cache-${{ env.RUN_MATRIX_COMBINATION }}-${{ hashFiles('tox.ini', + key: v4-tox-cache-${{ env.RUN_MATRIX_COMBINATION }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }}-core - name: Windows does not let git check out files with long names if: ${{ matrix.os == 'windows-2019'}} @@ -100,7 +107,7 @@ jobs: python-version: '3.10' architecture: 'x64' - name: Install tox - run: pip install tox==3.27.1 + run: pip install tox - name: Cache tox environment # Preserves .tox directory between runs for faster installs uses: actions/cache@v2 @@ -108,7 +115,7 @@ jobs: path: | .tox ~/.cache/pip - key: v3-tox-cache-${{ matrix.tox-environment }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') + key: v4-tox-cache-${{ matrix.tox-environment }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }}-core - name: run tox run: tox -e ${{ matrix.tox-environment }} @@ -169,7 +176,7 @@ jobs: - "tornado" - "tortoiseorm" - "urllib" - - "urllib3" + - "urllib3v" - "wsgi" - "prometheus-remote-write" - "richconsole" @@ -191,7 +198,7 @@ jobs: python-version: ${{ env[matrix.python-version] }} architecture: 'x64' - name: Install tox - run: pip install tox==3.27.1 -U tox-factor + run: pip install tox - name: Cache tox environment # Preserves .tox directory between runs for faster installs uses: actions/cache@v2 diff --git a/.pylintrc b/.pylintrc index 5b0f7862526..ab11620d772 100644 --- a/.pylintrc +++ b/.pylintrc @@ -76,6 +76,7 @@ disable=missing-docstring, unused-argument, # temp-pylint-upgrade redefined-builtin, cyclic-import, + broad-exception-raised, # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option @@ -480,10 +481,3 @@ max-statements=50 # Minimum number of public methods for a class (see R0903). min-public-methods=2 - - -[EXCEPTIONS] - -# Exceptions that will emit a warning when being caught. Defaults to -# "Exception". -overgeneral-exceptions=Exception diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d814cb8c48..a70ce75eaa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- Fix flush error when no LoggerProvider configured for LoggingHandler + ([#3608](https://github.com/open-telemetry/opentelemetry-python/pull/3608)) +- Fix `OTLPMetricExporter` ignores `preferred_aggregation` property + ([#3603](https://github.com/open-telemetry/opentelemetry-python/pull/3603)) +- Logs: set `observed_timestamp` field + ([#3565](https://github.com/open-telemetry/opentelemetry-python/pull/3565)) - Add missing Resource SchemaURL in OTLP exporters ([#3652](https://github.com/open-telemetry/opentelemetry-python/pull/3652)) - Fix loglevel warning text @@ -35,6 +41,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3645](https://github.com/open-telemetry/opentelemetry-python/pull/3645)) - Add Proxy classes for logging ([#3575](https://github.com/open-telemetry/opentelemetry-python/pull/3575)) +- Remove dependency on 'backoff' library + ([#3679](https://github.com/open-telemetry/opentelemetry-python/pull/3679)) ## Version 1.22.0/0.43b0 (2023-12-15) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 344b5585853..485cb6a0fcc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,14 +38,12 @@ during their normal contribution hours. This project uses [tox](https://tox.readthedocs.io) to automate some aspects of development, including testing against multiple Python versions. -To install `tox`, run[^1]: +To install `tox`, run: ```console -$ pip install tox==3.27.1 +$ pip install tox ``` -[^1]: Right now we are experiencing issues with `tox==4.x.y`, so we recommend you use this version. - You can run `tox` with the following arguments: - `tox` to run all existing tox commands, including unit tests for all packages diff --git a/dev-requirements.txt b/dev-requirements.txt index 11adfa75665..f440423ffcc 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -17,5 +17,5 @@ requests==2.31.0 ruamel.yaml==0.17.21 asgiref==3.7.2 psutil==5.9.6 -GitPython==3.1.40 +GitPython==3.1.41 flaky==3.7.0 diff --git a/docs/examples/fork-process-model/flask-gunicorn/requirements.txt b/docs/examples/fork-process-model/flask-gunicorn/requirements.txt index 0323bd5c5eb..ad166e35901 100644 --- a/docs/examples/fork-process-model/flask-gunicorn/requirements.txt +++ b/docs/examples/fork-process-model/flask-gunicorn/requirements.txt @@ -12,7 +12,7 @@ opentelemetry-instrumentation==0.41b0 opentelemetry-instrumentation-flask==0.41b0 opentelemetry-instrumentation-wsgi==0.41b0 opentelemetry-sdk==1.20.0 -protobuf==3.19.4 +protobuf==3.19.5 six==1.15.0 thrift==0.13.0 uWSGI==2.0.22 diff --git a/docs/examples/fork-process-model/flask-uwsgi/requirements.txt b/docs/examples/fork-process-model/flask-uwsgi/requirements.txt index 0323bd5c5eb..ad166e35901 100644 --- a/docs/examples/fork-process-model/flask-uwsgi/requirements.txt +++ b/docs/examples/fork-process-model/flask-uwsgi/requirements.txt @@ -12,7 +12,7 @@ opentelemetry-instrumentation==0.41b0 opentelemetry-instrumentation-flask==0.41b0 opentelemetry-instrumentation-wsgi==0.41b0 opentelemetry-sdk==1.20.0 -protobuf==3.19.4 +protobuf==3.19.5 six==1.15.0 thrift==0.13.0 uWSGI==2.0.22 diff --git a/docs/getting_started/tests/requirements.txt b/docs/getting_started/tests/requirements.txt index 962008c6488..79444476a25 100644 --- a/docs/getting_started/tests/requirements.txt +++ b/docs/getting_started/tests/requirements.txt @@ -10,7 +10,7 @@ idna==3.4 importlib-metadata==6.8.0 iniconfig==2.0.0 itsdangerous==2.1.2 -Jinja2==3.1.2 +Jinja2==3.1.3 MarkupSafe==2.1.3 packaging==23.2 pluggy==1.3.0 @@ -18,7 +18,7 @@ py==1.11.0 py-cpuinfo==9.0.0 pytest==7.1.3 pytest-benchmark==4.0.0 -requests==2.26.0 +requests==2.31.0 tomli==2.0.1 typing_extensions==4.8.0 urllib3==1.26.18 diff --git a/exporter/opentelemetry-exporter-otlp-proto-common/pyproject.toml b/exporter/opentelemetry-exporter-otlp-proto-common/pyproject.toml index e5b3084dbb8..979ffb87c86 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-common/pyproject.toml +++ b/exporter/opentelemetry-exporter-otlp-proto-common/pyproject.toml @@ -25,7 +25,6 @@ classifiers = [ ] dependencies = [ "opentelemetry-proto == 1.23.0.dev", - "backoff >= 1.10.0, < 3.0.0; python_version>='3.8'", ] [project.optional-dependencies] diff --git a/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/__init__.py index bd6ca4ad180..6593d89fd87 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/__init__.py @@ -15,9 +15,17 @@ import logging from collections.abc import Sequence -from typing import Any, Mapping, Optional, List, Callable, TypeVar, Dict - -import backoff +from itertools import count +from typing import ( + Any, + Mapping, + Optional, + List, + Callable, + TypeVar, + Dict, + Iterator, +) from opentelemetry.sdk.util.instrumentation import InstrumentationScope from opentelemetry.proto.common.v1.common_pb2 import ( @@ -37,7 +45,6 @@ from opentelemetry.sdk.trace import Resource from opentelemetry.util.types import Attributes - _logger = logging.getLogger(__name__) _TypingResourceT = TypeVar("_TypingResourceT") @@ -113,7 +120,6 @@ def _get_resource_data( resource_class: Callable[..., _TypingResourceT], name: str, ) -> List[_TypingResourceT]: - resource_data = [] for ( @@ -134,14 +140,36 @@ def _get_resource_data( return resource_data -# Work around API change between backoff 1.x and 2.x. Since 2.0.0 the backoff -# wait generator API requires a first .send(None) before reading the backoff -# values from the generator. -_is_backoff_v2 = next(backoff.expo()) is None - - -def _create_exp_backoff_generator(*args, **kwargs): - gen = backoff.expo(*args, **kwargs) - if _is_backoff_v2: - gen.send(None) - return gen +def _create_exp_backoff_generator(max_value: int = 0) -> Iterator[int]: + """ + Generates an infinite sequence of exponential backoff values. The sequence starts + from 1 (2^0) and doubles each time (2^1, 2^2, 2^3, ...). If a max_value is specified + and non-zero, the generated values will not exceed this maximum, capping at max_value + instead of growing indefinitely. + + Parameters: + - max_value (int, optional): The maximum value to yield. If 0 or not provided, the + sequence grows without bound. + + Returns: + Iterator[int]: An iterator that yields the exponential backoff values, either uncapped or + capped at max_value. + + Example: + ``` + gen = _create_exp_backoff_generator(max_value=10) + for _ in range(5): + print(next(gen)) + ``` + This will print: + 1 + 2 + 4 + 8 + 10 + + Note: this functionality used to be handled by the 'backoff' package. + """ + for i in count(0): + out = 2**i + yield min(out, max_value) if max_value else out diff --git a/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/metrics_encoder/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/metrics_encoder/__init__.py index ecd20b8145a..0d66fd28b70 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/metrics_encoder/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-common/src/opentelemetry/exporter/otlp/proto/common/_internal/metrics_encoder/__init__.py @@ -16,6 +16,7 @@ from opentelemetry.sdk.metrics.export import ( MetricExporter, ) +from opentelemetry.sdk.metrics.view import Aggregation from os import environ from opentelemetry.sdk.metrics import ( Counter, @@ -65,9 +66,18 @@ class OTLPMetricExporterMixin: def _common_configuration( self, preferred_temporality: Dict[type, AggregationTemporality] = None, + preferred_aggregation: Dict[type, Aggregation] = None, ) -> None: - instrument_class_temporality = {} + MetricExporter.__init__( + self, + preferred_temporality=self._get_temporality(preferred_temporality), + preferred_aggregation=self._get_aggregation(preferred_aggregation), + ) + + def _get_temporality( + self, preferred_temporality: Dict[type, AggregationTemporality] + ) -> Dict[type, AggregationTemporality]: otel_exporter_otlp_metrics_temporality_preference = ( environ.get( @@ -119,6 +129,13 @@ def _common_configuration( instrument_class_temporality.update(preferred_temporality or {}) + return instrument_class_temporality + + def _get_aggregation( + self, + preferred_aggregation: Dict[type, Aggregation], + ) -> Dict[type, Aggregation]: + otel_exporter_otlp_metrics_default_histogram_aggregation = environ.get( OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION, "explicit_bucket_histogram", @@ -128,7 +145,9 @@ def _common_configuration( "base2_exponential_bucket_histogram" ): - histogram_aggregation_type = ExponentialBucketHistogramAggregation + instrument_class_aggregation = { + Histogram: ExponentialBucketHistogramAggregation(), + } else: @@ -145,13 +164,13 @@ def _common_configuration( otel_exporter_otlp_metrics_default_histogram_aggregation, ) - histogram_aggregation_type = ExplicitBucketHistogramAggregation + instrument_class_aggregation = { + Histogram: ExplicitBucketHistogramAggregation(), + } - MetricExporter.__init__( - self, - preferred_temporality=instrument_class_temporality, - preferred_aggregation={Histogram: histogram_aggregation_type()}, - ) + instrument_class_aggregation.update(preferred_aggregation or {}) + + return instrument_class_aggregation def encode_metrics(data: MetricsData) -> ExportMetricsServiceRequest: diff --git a/exporter/opentelemetry-exporter-otlp-proto-common/tests/test_backoff.py b/exporter/opentelemetry-exporter-otlp-proto-common/tests/test_backoff.py new file mode 100644 index 00000000000..789a184ad04 --- /dev/null +++ b/exporter/opentelemetry-exporter-otlp-proto-common/tests/test_backoff.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.exporter.otlp.proto.common._internal import ( + _create_exp_backoff_generator, +) + + +class TestBackoffGenerator(TestCase): + def test_exp_backoff_generator(self): + generator = _create_exp_backoff_generator() + self.assertEqual(next(generator), 1) + self.assertEqual(next(generator), 2) + self.assertEqual(next(generator), 4) + self.assertEqual(next(generator), 8) + self.assertEqual(next(generator), 16) + + def test_exp_backoff_generator_with_max(self): + generator = _create_exp_backoff_generator(max_value=4) + self.assertEqual(next(generator), 1) + self.assertEqual(next(generator), 2) + self.assertEqual(next(generator), 4) + self.assertEqual(next(generator), 4) + self.assertEqual(next(generator), 4) + + def test_exp_backoff_generator_with_odd_max(self): + # use a max_value that's not in the set + generator = _create_exp_backoff_generator(max_value=11) + self.assertEqual(next(generator), 1) + self.assertEqual(next(generator), 2) + self.assertEqual(next(generator), 4) + self.assertEqual(next(generator), 8) + self.assertEqual(next(generator), 11) diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/pyproject.toml b/exporter/opentelemetry-exporter-otlp-proto-grpc/pyproject.toml index da67478b6ea..eeea4ea5174 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/pyproject.toml +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/pyproject.toml @@ -25,7 +25,6 @@ classifiers = [ ] dependencies = [ "Deprecated >= 1.2.6", - "backoff >= 1.8.0, < 3.0.0", "googleapis-common-protos ~= 1.52", "grpcio >= 1.0.0, < 2.0.0", "opentelemetry-api ~= 1.15", diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/metric_exporter/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/metric_exporter/__init__.py index 2560c5c3057..0ceca25c867 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/metric_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/src/opentelemetry/exporter/otlp/proto/grpc/metric_exporter/__init__.py @@ -127,7 +127,9 @@ def __init__( else compression ) - self._common_configuration(preferred_temporality) + self._common_configuration( + preferred_temporality, preferred_aggregation + ) OTLPExporterMixin.__init__( self, diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_metrics_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_metrics_exporter.py index 291e9457efd..95733b917bf 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_metrics_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_metrics_exporter.py @@ -968,6 +968,24 @@ def test_exponential_explicit_bucket_histogram(self): ExplicitBucketHistogramAggregation, ) + def test_preferred_aggregation_override(self): + + histogram_aggregation = ExplicitBucketHistogramAggregation( + boundaries=[0.05, 0.1, 0.5, 1, 5, 10], + ) + + exporter = OTLPMetricExporter( + preferred_aggregation={ + Histogram: histogram_aggregation, + }, + ) + + self.assertEqual( + # pylint: disable=protected-access + exporter._preferred_aggregation[Histogram], + histogram_aggregation, + ) + def _resource_metrics( index: int, scope_metrics: List[ScopeMetrics] diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py index 5445ddf9262..bb17e35b7b7 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py @@ -15,7 +15,6 @@ import os import threading import time -from collections import OrderedDict from concurrent.futures import ThreadPoolExecutor from logging import WARNING from unittest import TestCase @@ -28,7 +27,6 @@ from opentelemetry.attributes import BoundedAttributes from opentelemetry.exporter.otlp.proto.common._internal import ( _encode_key_value, - _is_backoff_v2, ) from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import ( OTLPSpanExporter, @@ -154,12 +152,12 @@ def setUp(self): "a", context=Mock( **{ - "trace_state": OrderedDict([("a", "b"), ("c", "d")]), + "trace_state": {"a": "b", "c": "d"}, "span_id": 10217189687419569865, "trace_id": 67545097771067222548457157018666467027, } ), - resource=SDKResource(OrderedDict([("a", 1), ("b", False)])), + resource=SDKResource({"a": 1, "b": False}), parent=Mock(**{"span_id": 12345}), attributes=BoundedAttributes(attributes={"a": 1, "b": True}), events=[event_mock], @@ -184,12 +182,12 @@ def setUp(self): "b", context=Mock( **{ - "trace_state": OrderedDict([("a", "b"), ("c", "d")]), + "trace_state": {"a": "b", "c": "d"}, "span_id": 10217189687419569865, "trace_id": 67545097771067222548457157018666467027, } ), - resource=SDKResource(OrderedDict([("a", 2), ("b", False)])), + resource=SDKResource({"a": 2, "b": False}), parent=Mock(**{"span_id": 12345}), instrumentation_scope=InstrumentationScope( name="name", version="version" @@ -200,12 +198,12 @@ def setUp(self): "c", context=Mock( **{ - "trace_state": OrderedDict([("a", "b"), ("c", "d")]), + "trace_state": {"a": "b", "c": "d"}, "span_id": 10217189687419569865, "trace_id": 67545097771067222548457157018666467027, } ), - resource=SDKResource(OrderedDict([("a", 1), ("b", False)])), + resource=SDKResource({"a": 1, "b": False}), parent=Mock(**{"span_id": 12345}), instrumentation_scope=InstrumentationScope( name="name2", version="version2" @@ -460,23 +458,6 @@ def test_otlp_headers(self, mock_ssl_channel, mock_secure): (("user-agent", "OTel-OTLP-Exporter-Python/" + __version__),), ) - @patch("opentelemetry.exporter.otlp.proto.common._internal.backoff") - @patch("opentelemetry.exporter.otlp.proto.grpc.exporter.sleep") - def test_handles_backoff_v2_api(self, mock_sleep, mock_backoff): - # In backoff ~= 2.0.0 the first value yielded from expo is None. - def generate_delays(*args, **kwargs): - if _is_backoff_v2: - yield None - yield 1 - - mock_backoff.expo.configure_mock(**{"side_effect": generate_delays}) - - add_TraceServiceServicer_to_server( - TraceServiceServicerUNAVAILABLE(), self.server - ) - self.exporter.export([self.span]) - mock_sleep.assert_called_once_with(1) - @patch( "opentelemetry.exporter.otlp.proto.grpc.exporter._create_exp_backoff_generator" ) @@ -982,7 +963,7 @@ def _create_span_with_status(status: SDKStatus): "a", context=Mock( **{ - "trace_state": OrderedDict([("a", "b"), ("c", "d")]), + "trace_state": {"a": "b", "c": "d"}, "span_id": 10217189687419569865, "trace_id": 67545097771067222548457157018666467027, } diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/pyproject.toml b/exporter/opentelemetry-exporter-otlp-proto-http/pyproject.toml index 35c4ca24a3c..740f05c3cfe 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/pyproject.toml +++ b/exporter/opentelemetry-exporter-otlp-proto-http/pyproject.toml @@ -25,7 +25,6 @@ classifiers = [ ] dependencies = [ "Deprecated >= 1.2.6", - "backoff >= 1.10.0, < 3.0.0", "googleapis-common-protos ~= 1.52", "opentelemetry-api ~= 1.15", "opentelemetry-proto == 1.23.0.dev", diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/metric_exporter/__init__.py b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/metric_exporter/__init__.py index becdab257fe..6be74a37a06 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/metric_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/metric_exporter/__init__.py @@ -135,7 +135,9 @@ def __init__( {"Content-Encoding": self._compression.value} ) - self._common_configuration(preferred_temporality) + self._common_configuration( + preferred_temporality, preferred_aggregation + ) def _export(self, serialized_data: str): data = serialized_data diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py index c06b5db3c22..674785056a5 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/tests/metrics/test_otlp_metrics_exporter.py @@ -15,13 +15,12 @@ from logging import WARNING from os import environ from unittest import TestCase -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import MagicMock, Mock, call, patch from requests import Session from requests.models import Response from responses import POST, activate, add -from opentelemetry.exporter.otlp.proto.common._internal import _is_backoff_v2 from opentelemetry.exporter.otlp.proto.common.metrics_encoder import ( encode_metrics, ) @@ -298,17 +297,8 @@ def test_serialization(self, mock_post): ) @activate - @patch("opentelemetry.exporter.otlp.proto.common._internal.backoff") @patch("opentelemetry.exporter.otlp.proto.http.metric_exporter.sleep") - def test_handles_backoff_v2_api(self, mock_sleep, mock_backoff): - # In backoff ~= 2.0.0 the first value yielded from expo is None. - def generate_delays(*args, **kwargs): - if _is_backoff_v2: - yield None - yield 1 - - mock_backoff.expo.configure_mock(**{"side_effect": generate_delays}) - + def test_exponential_backoff(self, mock_sleep): # return a retryable error add( POST, @@ -323,7 +313,9 @@ def generate_delays(*args, **kwargs): metrics_data = self.metrics["sum_int"] exporter.export(metrics_data) - mock_sleep.assert_called_once_with(1) + mock_sleep.assert_has_calls( + [call(1), call(2), call(4), call(8), call(16), call(32)] + ) def test_aggregation_temporality(self): @@ -487,3 +479,19 @@ def test_2xx_status_code(self, mock_otlp_metric_exporter): OTLPMetricExporter().export(MagicMock()), MetricExportResult.SUCCESS, ) + + def test_preferred_aggregation_override(self): + + histogram_aggregation = ExplicitBucketHistogramAggregation( + boundaries=[0.05, 0.1, 0.5, 1, 5, 10], + ) + + exporter = OTLPMetricExporter( + preferred_aggregation={ + Histogram: histogram_aggregation, + }, + ) + + self.assertEqual( + exporter._preferred_aggregation[Histogram], histogram_aggregation + ) diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py index e601e5d00cb..6b6aafd465f 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_log_exporter.py @@ -16,13 +16,12 @@ import unittest from typing import List -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import MagicMock, Mock, call, patch import requests import responses from opentelemetry._logs import SeverityNumber -from opentelemetry.exporter.otlp.proto.common._internal import _is_backoff_v2 from opentelemetry.exporter.otlp.proto.http import Compression from opentelemetry.exporter.otlp.proto.http._log_exporter import ( DEFAULT_COMPRESSION, @@ -169,17 +168,8 @@ def test_exporter_env(self): self.assertIsInstance(exporter._session, requests.Session) @responses.activate - @patch("opentelemetry.exporter.otlp.proto.common._internal.backoff") @patch("opentelemetry.exporter.otlp.proto.http._log_exporter.sleep") - def test_handles_backoff_v2_api(self, mock_sleep, mock_backoff): - # In backoff ~= 2.0.0 the first value yielded from expo is None. - def generate_delays(*args, **kwargs): - if _is_backoff_v2: - yield None - yield 1 - - mock_backoff.expo.configure_mock(**{"side_effect": generate_delays}) - + def test_exponential_backoff(self, mock_sleep): # return a retryable error responses.add( responses.POST, @@ -192,7 +182,9 @@ def generate_delays(*args, **kwargs): logs = self._get_sdk_log_data() exporter.export(logs) - mock_sleep.assert_called_once_with(1) + mock_sleep.assert_has_calls( + [call(1), call(2), call(4), call(8), call(16), call(32)] + ) @staticmethod def _get_sdk_log_data() -> List[LogData]: diff --git a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py index eb5b375e40d..69874664c7a 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-http/tests/test_proto_span_exporter.py @@ -13,13 +13,11 @@ # limitations under the License. import unittest -from collections import OrderedDict -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import MagicMock, Mock, call, patch import requests import responses -from opentelemetry.exporter.otlp.proto.common._internal import _is_backoff_v2 from opentelemetry.exporter.otlp.proto.http import Compression from opentelemetry.exporter.otlp.proto.http.trace_exporter import ( DEFAULT_COMPRESSION, @@ -205,17 +203,8 @@ def test_headers_parse_from_env(self): # pylint: disable=no-self-use @responses.activate - @patch("opentelemetry.exporter.otlp.proto.common._internal.backoff") @patch("opentelemetry.exporter.otlp.proto.http.trace_exporter.sleep") - def test_handles_backoff_v2_api(self, mock_sleep, mock_backoff): - # In backoff ~= 2.0.0 the first value yielded from expo is None. - def generate_delays(*args, **kwargs): - if _is_backoff_v2: - yield None - yield 1 - - mock_backoff.expo.configure_mock(**{"side_effect": generate_delays}) - + def test_exponential_backoff(self, mock_sleep): # return a retryable error responses.add( responses.POST, @@ -231,7 +220,7 @@ def generate_delays(*args, **kwargs): "abc", context=Mock( **{ - "trace_state": OrderedDict([("a", "b"), ("c", "d")]), + "trace_state": {"a": "b", "c": "d"}, "span_id": 10217189687419569865, "trace_id": 67545097771067222548457157018666467027, } @@ -239,7 +228,9 @@ def generate_delays(*args, **kwargs): ) exporter.export([span]) - mock_sleep.assert_called_once_with(1) + mock_sleep.assert_has_calls( + [call(1), call(2), call(4), call(8), call(16), call(32)] + ) @patch.object(OTLPSpanExporter, "_export", return_value=Mock(ok=True)) def test_2xx_status_code(self, mock_otlp_metric_exporter): diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index 783d18163ec..89c879459f1 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -148,7 +148,8 @@ def __init__( self.maxlen = maxlen self.dropped = 0 self.max_value_len = max_value_len - self._dict = OrderedDict() # type: OrderedDict + # OrderedDict is not used until the maxlen is reached for efficiency. + self._dict = {} # type: dict | OrderedDict self._lock = threading.Lock() # type: threading.Lock if attributes: for key, value in attributes.items(): @@ -178,6 +179,8 @@ def __setitem__(self, key, value): elif ( self.maxlen is not None and len(self._dict) == self.maxlen ): + if not isinstance(self._dict, OrderedDict): + self._dict = OrderedDict(self._dict) self._dict.popitem(last=False) self.dropped += 1 diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index 805b2b06b18..327d85fef48 100644 --- a/opentelemetry-api/src/opentelemetry/trace/span.py +++ b/opentelemetry-api/src/opentelemetry/trace/span.py @@ -3,7 +3,6 @@ import re import types as python_types import typing -from collections import OrderedDict from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util import types @@ -218,7 +217,7 @@ def __init__( typing.Sequence[typing.Tuple[str, str]] ] = None, ) -> None: - self._dict = OrderedDict() # type: OrderedDict[str, str] + self._dict = {} # type: dict[str, str] if entries is None: return if len(entries) > _TRACECONTEXT_MAXIMUM_TRACESTATE_KEYS: @@ -310,9 +309,8 @@ def update(self, key: str, value: str) -> "TraceState": ) return self prev_state = self._dict.copy() - prev_state[key] = value - prev_state.move_to_end(key, last=False) - new_state = list(prev_state.items()) + prev_state.pop(key, None) + new_state = [(key, value), *prev_state.items()] return TraceState(new_state) def delete(self, key: str) -> "TraceState": @@ -362,7 +360,7 @@ def from_header(cls, header_list: typing.List[str]) -> "TraceState": If the number of keys is beyond the maximum, all values will be discarded and an empty tracestate will be returned. """ - pairs = OrderedDict() # type: OrderedDict[str, str] + pairs = {} # type: dict[str, str] for header in header_list: members: typing.List[str] = re.split(_delimiter_pattern, header) for member in members: diff --git a/opentelemetry-api/tests/attributes/test_attributes.py b/opentelemetry-api/tests/attributes/test_attributes.py index 121dec3d251..ad2f741fb1f 100644 --- a/opentelemetry-api/tests/attributes/test_attributes.py +++ b/opentelemetry-api/tests/attributes/test_attributes.py @@ -14,7 +14,6 @@ # type: ignore -import collections import unittest from typing import MutableSequence @@ -90,14 +89,12 @@ def test_sequence_attr_decode(self): class TestBoundedAttributes(unittest.TestCase): - base = collections.OrderedDict( - [ - ("name", "Firulais"), - ("age", 7), - ("weight", 13), - ("vaccinated", True), - ] - ) + base = { + "name": "Firulais", + "age": 7, + "weight": 13, + "vaccinated": True, + } def test_negative_maxlen(self): with self.assertRaises(ValueError): @@ -105,7 +102,7 @@ def test_negative_maxlen(self): def test_from_map(self): dic_len = len(self.base) - base_copy = collections.OrderedDict(self.base) + base_copy = self.base.copy() bdict = BoundedAttributes(dic_len, base_copy) self.assertEqual(len(bdict), dic_len) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index 0e85b464c33..783a99a7d51 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -466,6 +466,7 @@ def _get_attributes(self, record: logging.LogRecord) -> Attributes: def _translate(self, record: logging.LogRecord) -> LogRecord: timestamp = int(record.created * 1e9) + observered_timestamp = time_ns() span_context = get_current_span().get_span_context() attributes = self._get_attributes(record) severity_number = std_to_otel(record.levelno) @@ -479,6 +480,7 @@ def _translate(self, record: logging.LogRecord) -> LogRecord: return LogRecord( timestamp=timestamp, + observed_timestamp=observered_timestamp, trace_id=span_context.trace_id, span_id=span_context.span_id, trace_flags=span_context.trace_flags, @@ -500,9 +502,10 @@ def emit(self, record: logging.LogRecord) -> None: def flush(self) -> None: """ - Flushes the logging output. + Flushes the logging output. Skip flushing if logger is NoOp. """ - self._logger_provider.force_flush() + if not isinstance(self._logger, NoOpLogger): + self._logger_provider.force_flush() class Logger(APILogger): diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 6dae70b2f6b..344838ba186 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -1042,8 +1042,8 @@ def start_as_current_span( end_on_exit=end_on_exit, record_exception=record_exception, set_status_on_exception=set_status_on_exception, - ) as span_context: - yield span_context + ) as span: + yield span def start_span( # pylint: disable=too-many-locals self, diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py index e1857d8e62d..37ca62b017c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py @@ -14,7 +14,7 @@ import datetime import threading -from collections import OrderedDict, deque +from collections import deque from collections.abc import MutableMapping, Sequence from typing import Optional @@ -107,7 +107,7 @@ def __init__(self, maxlen: Optional[int]): raise ValueError self.maxlen = maxlen self.dropped = 0 - self._dict = OrderedDict() # type: OrderedDict + self._dict = {} # type: dict self._lock = threading.Lock() # type: threading.Lock def __repr__(self): @@ -143,7 +143,7 @@ def __len__(self): @classmethod def from_map(cls, maxlen, mapping): - mapping = OrderedDict(mapping) + mapping = dict(mapping) bounded_dict = cls(maxlen) for key, value in mapping.items(): bounded_dict[key] = value diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 2bd82efd464..7a32a3ed75c 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -81,6 +81,19 @@ def test_log_record_emit_noop(self): logger.warning("Warning message") handler_mock._translate.assert_not_called() + def test_log_flush_noop(self): + + no_op_logger_provider = NoOpLoggerProvider() + no_op_logger_provider.force_flush = Mock() + + logger = get_logger(logger_provider=no_op_logger_provider) + + with self.assertLogs(level=logging.WARNING): + logger.warning("Warning message") + + logger.handlers[0].flush() + no_op_logger_provider.force_flush.assert_not_called() + def test_log_record_no_span_context(self): emitter_provider_mock = Mock(spec=LoggerProvider) emitter_mock = APIGetLogger( @@ -100,6 +113,20 @@ def test_log_record_no_span_context(self): log_record.trace_flags, INVALID_SPAN_CONTEXT.trace_flags ) + def test_log_record_observed_timestamp(self): + emitter_provider_mock = Mock(spec=LoggerProvider) + emitter_mock = APIGetLogger( + __name__, logger_provider=emitter_provider_mock + ) + logger = get_logger(logger_provider=emitter_provider_mock) + # Assert emit gets called for warning message + with self.assertLogs(level=logging.WARNING): + logger.warning("Warning message") + args, _ = emitter_mock.emit.call_args_list[0] + log_record = args[0] + + self.assertIsNotNone(log_record.observed_timestamp) + def test_log_record_user_attributes(self): """Attributes can be injected into logs by adding them to the LogRecord""" emitter_provider_mock = Mock(spec=LoggerProvider) diff --git a/tox.ini b/tox.ini index 035f8269e37..1bd0c98226e 100644 --- a/tox.ini +++ b/tox.ini @@ -20,6 +20,7 @@ envlist = ; docs/getting-started py3{8,9,10,11}-opentelemetry-getting-started + pypy3-opentelemetry-getting-started py3{8,9,10,11}-opentelemetry-opentracing-shim pypy3-opentelemetry-opentracing-shim @@ -31,6 +32,7 @@ envlist = ; exporter-opencensus intentionally excluded from pypy3 py3{8,9,10,11}-proto{3,4}-opentelemetry-exporter-otlp-proto-common + pypy3-proto{3,4}-opentelemetry-exporter-otlp-proto-common ; opentelemetry-exporter-otlp py3{8,9,10,11}-opentelemetry-exporter-otlp-combined @@ -90,8 +92,8 @@ deps = setenv = ; override CONTRIB_REPO_SHA via env variable when testing other branches/commits than main ; 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}" + 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/:{toxinidir}/tests/opentelemetry-test-utils/src/ changedir = @@ -127,11 +129,11 @@ commands_pre = protobuf: pip install {toxinidir}/opentelemetry-proto getting-started: pip install -r requirements.txt - getting-started: pip install -e "{env:CONTRIB_REPO}#egg=opentelemetry-util-http&subdirectory=util/opentelemetry-util-http" - getting-started: pip install -e "{env:CONTRIB_REPO}#egg=opentelemetry-instrumentation&subdirectory=opentelemetry-instrumentation" - getting-started: pip install -e "{env:CONTRIB_REPO}#egg=opentelemetry-instrumentation-requests&subdirectory=instrumentation/opentelemetry-instrumentation-requests" - getting-started: pip install -e "{env:CONTRIB_REPO}#egg=opentelemetry-instrumentation-wsgi&subdirectory=instrumentation/opentelemetry-instrumentation-wsgi" - getting-started: pip install -e "{env:CONTRIB_REPO}#egg=opentelemetry-instrumentation-flask&subdirectory=instrumentation/opentelemetry-instrumentation-flask" + getting-started: pip install -e {env:CONTRIB_REPO}\#egg=opentelemetry-util-http&subdirectory=util/opentelemetry-util-http + getting-started: pip install -e {env:CONTRIB_REPO}\#egg=opentelemetry-instrumentation&subdirectory=opentelemetry-instrumentation + getting-started: pip install -e {env:CONTRIB_REPO}\#egg=opentelemetry-instrumentation-requests&subdirectory=instrumentation/opentelemetry-instrumentation-requests + getting-started: pip install -e {env:CONTRIB_REPO}\#egg=opentelemetry-instrumentation-wsgi&subdirectory=instrumentation/opentelemetry-instrumentation-wsgi + getting-started: pip install -e {env:CONTRIB_REPO}\#egg=opentelemetry-instrumentation-flask&subdirectory=instrumentation/opentelemetry-instrumentation-flask opencensus: pip install {toxinidir}/exporter/opentelemetry-exporter-opencensus @@ -255,14 +257,17 @@ deps = requests~=2.7 markupsafe~=2.1 +allowlist_externals = + {toxinidir}/scripts/tracecontext-integration-test.sh + commands_pre = pip install -e {toxinidir}/opentelemetry-api \ -e {toxinidir}/opentelemetry-semantic-conventions \ -e {toxinidir}/opentelemetry-sdk \ - -e "{env:CONTRIB_REPO}#egg=opentelemetry-util-http&subdirectory=util/opentelemetry-util-http" \ - -e "{env:CONTRIB_REPO}#egg=opentelemetry-instrumentation&subdirectory=opentelemetry-instrumentation" \ - -e "{env:CONTRIB_REPO}#egg=opentelemetry-instrumentation-requests&subdirectory=instrumentation/opentelemetry-instrumentation-requests" \ - -e "{env:CONTRIB_REPO}#egg=opentelemetry-instrumentation-wsgi&subdirectory=instrumentation/opentelemetry-instrumentation-wsgi" + -e {env:CONTRIB_REPO}\#egg=opentelemetry-util-http&subdirectory=util/opentelemetry-util-http \ + -e {env:CONTRIB_REPO}\#egg=opentelemetry-instrumentation&subdirectory=opentelemetry-instrumentation \ + -e {env:CONTRIB_REPO}\#egg=opentelemetry-instrumentation-requests&subdirectory=instrumentation/opentelemetry-instrumentation-requests \ + -e {env:CONTRIB_REPO}\#egg=opentelemetry-instrumentation-wsgi&subdirectory=instrumentation/opentelemetry-instrumentation-wsgi commands = {toxinidir}/scripts/tracecontext-integration-test.sh From 169931bbad01fd10a19597f1fc42f485bac8b5c4 Mon Sep 17 00:00:00 2001 From: Arne Date: Thu, 22 Feb 2024 21:25:04 +0100 Subject: [PATCH 07/15] fix: mollify linter --- .../src/opentelemetry/sdk/_logs/_internal/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index 783a99a7d51..1f379e217ec 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -436,7 +436,8 @@ def __init__( __name__, logger_provider=self._logger_provider ) - def _get_attributes(self, record: logging.LogRecord) -> Attributes: + @staticmethod + def _get_attributes(record: logging.LogRecord) -> Attributes: attributes = { k: v for k, v in vars(record).items() From 05525906286ae668a497796234ad61e9cffd24d1 Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Thu, 4 Jul 2024 10:59:10 +0200 Subject: [PATCH 08/15] fix: roll back changes to the dev-setup that somehow only worked this way on my end --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index c66e0612ca0..588b7ccb399 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,8 @@ this repository and perform an [editable install](https://pip.pypa.io/en/stable/reference/pip_install/#editable-installs): ```sh -pip install -e ./opentelemetry-sdk/ -e ./opentelemetry-api/ -e ./opentelemetry-semantic-conventions/ +pip install -e ./opentelemetry-api +pip install -e ./opentelemetry-sdk pip install -e ./instrumentation/opentelemetry-instrumentation-{instrumentation} ``` From 79ce99aea6ab00ccf8ab75bad5b6e2844de0b6ba Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Thu, 4 Jul 2024 11:13:36 +0200 Subject: [PATCH 09/15] fix: re-enabled skipping over native logrecord attributes again that aren't covered by a semantic convention and ran black --- .../opentelemetry/sdk/_logs/_internal/__init__.py | 15 ++++++++++++++- opentelemetry-sdk/tests/logs/test_handler.py | 14 ++------------ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index 390f2471c18..9b7d4a426f0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -57,7 +57,7 @@ _DEFAULT_OTEL_ATTRIBUTE_COUNT_LIMIT = 128 _ENV_VALUE_UNSET = "" _EXCLUDED_ATTRIBUTES = { - # pseudo-private log-record attributes, they just get dropped + # pseudo-private log-record attributes, they should always be dropped "args", "msg", "message", @@ -71,6 +71,19 @@ "lineno", "thread", "threadName", + # attributes that are omitted because no semantic convention exists for them yet + "asctime", + "created", + "filename", + "levelname", + "levelno", + "module", + "msecs", + "name", + "process", + "processName", + "relativeCreated", + "taskName", } diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 7a32a3ed75c..0d5d0891c1d 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -255,20 +255,10 @@ def test_log_record_args_are_translated(self): self.assertEqual( set(log_record.attributes), { + "thread.id", "code.filepath", - "code.function", "code.lineno", - "created", - "filename", - "levelname", - "levelno", - "module", - "msecs", - "name", - "process", - "processName", - "relativeCreated", - "thread.id", + "code.function", "thread.name", }, ) From cbc0920632d68df647c76aba882644259ed284b1 Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Wed, 28 Aug 2024 11:35:01 +0200 Subject: [PATCH 10/15] fix: add tests for percent-style logging calls to ensure they're still possible --- opentelemetry-sdk/tests/logs/test_handler.py | 312 ++++++++++--------- 1 file changed, 160 insertions(+), 152 deletions(-) diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 35e2e489b9c..10eb12d3d2c 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -13,7 +13,8 @@ # limitations under the License. import logging import unittest -from unittest.mock import Mock +from unittest.mock import Mock, MagicMock +from contextlib import contextmanager from opentelemetry._logs import NoOpLoggerProvider, SeverityNumber from opentelemetry._logs import get_logger as APIGetLogger @@ -31,30 +32,28 @@ class TestLoggingHandler(unittest.TestCase): def test_handler_default_log_level(self): - processor, logger = set_up_test_logging(logging.NOTSET) + with set_up_test_logging(logging.NOTSET) as (processor, logger): + # Make sure debug messages are ignored by default + logger.debug("Debug message") + assert processor.emit_count() == 0 - # Make sure debug messages are ignored by default - logger.debug("Debug message") - assert processor.emit_count() == 0 - - # Assert emit gets called for warning message - with self.assertLogs(level=logging.WARNING): - logger.warning("Warning message") - self.assertEqual(processor.emit_count(), 1) + # Assert emit gets called for warning message + with self.assertLogs(level=logging.WARNING): + logger.warning("Warning message") + self.assertEqual(processor.emit_count(), 1) def test_handler_custom_log_level(self): - processor, logger = set_up_test_logging(logging.ERROR) - - with self.assertLogs(level=logging.WARNING): - logger.warning("Warning message test custom log level") - # Make sure any log with level < ERROR is ignored - assert processor.emit_count() == 0 + with set_up_test_logging(logging.ERROR) as (processor, logger): + with self.assertLogs(level=logging.WARNING): + logger.warning("Warning message test custom log level") + # Make sure any log with level < ERROR is ignored + assert processor.emit_count() == 0 - with self.assertLogs(level=logging.ERROR): - logger.error("Mumbai, we have a major problem") - with self.assertLogs(level=logging.CRITICAL): - logger.critical("No Time For Caution") - self.assertEqual(processor.emit_count(), 2) + with self.assertLogs(level=logging.ERROR): + logger.error("Mumbai, we have a major problem") + with self.assertLogs(level=logging.CRITICAL): + logger.critical("No Time For Caution") + self.assertEqual(processor.emit_count(), 2) # pylint: disable=protected-access def test_log_record_emit_noop(self): @@ -70,6 +69,9 @@ def test_log_record_emit_noop(self): with self.assertLogs(level=logging.WARNING): logger.warning("Warning message") + # teardown + logger.removeHandler(handler_mock) + def test_log_flush_noop(self): no_op_logger_provider = NoOpLoggerProvider() no_op_logger_provider.force_flush = Mock() @@ -86,173 +88,178 @@ def test_log_flush_noop(self): logger.handlers[0].flush() no_op_logger_provider.force_flush.assert_not_called() - def test_log_record_no_span_context(self): - processor, logger = set_up_test_logging(logging.WARNING) + # teardown + logger.removeHandler(handler) - # Assert emit gets called for warning message - with self.assertLogs(level=logging.WARNING): - logger.warning("Warning message") + def test_log_record_no_span_context(self): + with set_up_test_logging(logging.WARNING) as (processor, logger): + # Assert emit gets called for warning message + with self.assertLogs(level=logging.WARNING): + logger.warning("Warning message") - log_record = processor.get_log_record(0) + log_record = processor.get_log_record(0) - self.assertIsNotNone(log_record) - self.assertEqual(log_record.trace_id, INVALID_SPAN_CONTEXT.trace_id) - self.assertEqual(log_record.span_id, INVALID_SPAN_CONTEXT.span_id) - self.assertEqual( - log_record.trace_flags, INVALID_SPAN_CONTEXT.trace_flags - ) + self.assertIsNotNone(log_record) + self.assertEqual(log_record.trace_id, INVALID_SPAN_CONTEXT.trace_id) + self.assertEqual(log_record.span_id, INVALID_SPAN_CONTEXT.span_id) + self.assertEqual( + log_record.trace_flags, INVALID_SPAN_CONTEXT.trace_flags + ) def test_log_record_observed_timestamp(self): - processor, logger = set_up_test_logging(logging.WARNING) + with set_up_test_logging(logging.WARNING) as (processor, logger): + with self.assertLogs(level=logging.WARNING): + logger.warning("Warning message") - with self.assertLogs(level=logging.WARNING): - logger.warning("Warning message") - - log_record = processor.get_log_record(0) - self.assertIsNotNone(log_record.observed_timestamp) + log_record = processor.get_log_record(0) + self.assertIsNotNone(log_record.observed_timestamp) def test_log_record_user_attributes(self): """Attributes can be injected into logs by adding them to the LogRecord""" - processor, logger = set_up_test_logging(logging.WARNING) + with set_up_test_logging(logging.WARNING) as (processor, logger): + # Assert emit gets called for warning message + with self.assertLogs(level=logging.WARNING): + logger.warning("Warning message", extra={"http.status_code": 200}) - # Assert emit gets called for warning message - with self.assertLogs(level=logging.WARNING): - logger.warning("Warning message", extra={"http.status_code": 200}) - - log_record = processor.get_log_record(0) + log_record = processor.get_log_record(0) - self.assertIsNotNone(log_record) - self.assertEqual( - log_record.attributes, - {**log_record.attributes, **{"http.status_code": 200}}, - ) - self.assertTrue( - log_record.attributes[SpanAttributes.CODE_FILEPATH].endswith( - "test_handler.py" + self.assertIsNotNone(log_record) + self.assertEqual( + log_record.attributes, + {**log_record.attributes, "http.status_code": 200}, ) - ) - self.assertEqual( - log_record.attributes[SpanAttributes.CODE_FUNCTION], - "test_log_record_user_attributes", - ) - # The line of the log statement is not a constant (changing tests may change that), - # so only check that the attribute is present. - self.assertTrue(SpanAttributes.CODE_LINENO in log_record.attributes) - self.assertTrue(isinstance(log_record.attributes, BoundedAttributes)) + self.assertTrue( + log_record.attributes[SpanAttributes.CODE_FILEPATH].endswith( + "test_handler.py" + ) + ) + self.assertEqual( + log_record.attributes[SpanAttributes.CODE_FUNCTION], + "test_log_record_user_attributes", + ) + # The line of the log statement is not a constant (changing tests may change that), + # so only check that the attribute is present. + self.assertTrue(SpanAttributes.CODE_LINENO in log_record.attributes) + self.assertTrue(isinstance(log_record.attributes, BoundedAttributes)) def test_log_record_exception(self): """Exception information will be included in attributes""" - processor, logger = set_up_test_logging(logging.ERROR) - - try: - raise ZeroDivisionError("division by zero") - except ZeroDivisionError: - with self.assertLogs(level=logging.ERROR): - logger.exception("Zero Division Error") + with set_up_test_logging(logging.ERROR) as (processor, logger): + try: + raise ZeroDivisionError("division by zero") + except ZeroDivisionError: + with self.assertLogs(level=logging.ERROR): + logger.exception("Zero Division Error") - log_record = processor.get_log_record(0) + log_record = processor.get_log_record(0) - self.assertIsNotNone(log_record) - self.assertIn("Zero Division Error", log_record.body) - self.assertEqual( - log_record.attributes[SpanAttributes.EXCEPTION_TYPE], - ZeroDivisionError.__name__, - ) - self.assertEqual( - log_record.attributes[SpanAttributes.EXCEPTION_MESSAGE], - "division by zero", - ) - stack_trace = log_record.attributes[ - SpanAttributes.EXCEPTION_STACKTRACE - ] - self.assertIsInstance(stack_trace, str) - self.assertTrue("Traceback" in stack_trace) - self.assertTrue("ZeroDivisionError" in stack_trace) - self.assertTrue("division by zero" in stack_trace) - self.assertTrue(__file__ in stack_trace) + self.assertIsNotNone(log_record) + self.assertIn("Zero Division Error", log_record.body) + self.assertEqual( + log_record.attributes[SpanAttributes.EXCEPTION_TYPE], + ZeroDivisionError.__name__, + ) + self.assertEqual( + log_record.attributes[SpanAttributes.EXCEPTION_MESSAGE], + "division by zero", + ) + stack_trace = log_record.attributes[ + SpanAttributes.EXCEPTION_STACKTRACE + ] + self.assertIsInstance(stack_trace, str) + self.assertTrue("Traceback" in stack_trace) + self.assertTrue("ZeroDivisionError" in stack_trace) + self.assertTrue("division by zero" in stack_trace) + self.assertTrue(__file__ in stack_trace) def test_log_exc_info_false(self): """Exception information will be included in attributes""" - processor, logger = set_up_test_logging(logging.NOTSET) - - try: - raise ZeroDivisionError("division by zero") - except ZeroDivisionError: - with self.assertLogs(level=logging.ERROR): - logger.error("Zero Division Error", exc_info=False) + with set_up_test_logging(logging.NOTSET) as (processor, logger): + try: + raise ZeroDivisionError("division by zero") + except ZeroDivisionError: + with self.assertLogs(level=logging.ERROR): + logger.error("Zero Division Error", exc_info=False) - log_record = processor.get_log_record(0) + log_record = processor.get_log_record(0) - self.assertIsNotNone(log_record) - self.assertEqual(log_record.body, "Zero Division Error") - self.assertNotIn(SpanAttributes.EXCEPTION_TYPE, log_record.attributes) - self.assertNotIn( - SpanAttributes.EXCEPTION_MESSAGE, log_record.attributes - ) - self.assertNotIn( - SpanAttributes.EXCEPTION_STACKTRACE, log_record.attributes - ) + self.assertIsNotNone(log_record) + self.assertEqual(log_record.body, "Zero Division Error") + self.assertNotIn(SpanAttributes.EXCEPTION_TYPE, log_record.attributes) + self.assertNotIn( + SpanAttributes.EXCEPTION_MESSAGE, log_record.attributes + ) + self.assertNotIn( + SpanAttributes.EXCEPTION_STACKTRACE, log_record.attributes + ) def test_log_record_trace_correlation(self): - processor, logger = set_up_test_logging(logging.WARNING) + with set_up_test_logging(logging.WARNING) as (processor, logger): + tracer = trace.TracerProvider().get_tracer(__name__) + with tracer.start_as_current_span("test") as span: + with self.assertLogs(level=logging.CRITICAL): + logger.critical("Critical message within span") + + log_record = processor.get_log_record(0) + + self.assertEqual(log_record.body, "Critical message within span") + self.assertEqual(log_record.severity_text, "CRITICAL") + self.assertEqual(log_record.severity_number, SeverityNumber.FATAL) + span_context = span.get_span_context() + self.assertEqual(log_record.trace_id, span_context.trace_id) + self.assertEqual(log_record.span_id, span_context.span_id) + self.assertEqual(log_record.trace_flags, span_context.trace_flags) - tracer = trace.TracerProvider().get_tracer(__name__) - with tracer.start_as_current_span("test") as span: - with self.assertLogs(level=logging.CRITICAL): - logger.critical("Critical message within span") + def test_log_record_args_are_translated(self): + with set_up_test_logging(logging.WARNING) as (processor, logger): + logger.warning("Test message") log_record = processor.get_log_record(0) + self.assertEqual( + set(log_record.attributes), + { + "code.filepath", + "code.lineno", + "code.function", + }, + ) - self.assertEqual(log_record.body, "Critical message within span") - self.assertEqual(log_record.severity_text, "CRITICAL") - self.assertEqual(log_record.severity_number, SeverityNumber.FATAL) - span_context = span.get_span_context() - self.assertEqual(log_record.trace_id, span_context.trace_id) - self.assertEqual(log_record.span_id, span_context.span_id) - self.assertEqual(log_record.trace_flags, span_context.trace_flags) + def test_format_is_called(self): + with set_up_test_logging(logging.WARNING, formatter=logging.Formatter("%(name)s - %(levelname)s - %(message)s")) as (processor, logger): + logger.warning("Test message") - def test_log_record_args_are_translated(self): - processor, logger = set_up_test_logging(logging.INFO) - - with self.assertLogs(level=logging.INFO): - logger.info("Test message") - - log_record = processor.get_log_record(0) - self.assertEqual( - set(log_record.attributes), - { - "thread.id", - "code.filepath", - "code.lineno", - "code.function", - "thread.name", - }, - ) + log_record = processor.get_log_record(0) + self.assertEqual( + log_record.body, "foo - WARNING - Test message" + ) - def test_format_is_called(self): - processor, logger = set_up_test_logging( - logging.INFO, - logging.Formatter("%(name)s - %(levelname)s - %(message)s") - ) + def test_log_body_is_always_string(self): + with set_up_test_logging(logging.WARNING) as (processor, logger): + logger.warning(["something", "of", "note"]) - with self.assertLogs(level=logging.INFO): - logger.info("Test message") + log_record = processor.get_log_record(0) + self.assertIsInstance(log_record.body, str) - log_record = processor.get_log_record(0) - self.assertEqual( - log_record.body, "foo - INFO - Test message" - ) + def test_args_get_transformed_if_logged(self): + with set_up_test_logging(logging.WARNING) as (_, logger): - def test_log_body_is_always_string(self): - processor, logger = set_up_test_logging(logging.INFO) + my_object = MagicMock() + logger.warning("%s - %d", my_object, my_object) + + self.assertTrue(my_object.__str__.called) + self.assertTrue(my_object.__int__.called) + + def test_args_do_not_get_transformed_if_not_logged(self): + with set_up_test_logging(logging.WARNING) as (_, logger): - with self.assertLogs(level=logging.INFO): - logger.info(["something", "of", "note"]) + my_object = MagicMock() + logger.info("%s - %d", my_object, my_object) - log_record = processor.get_log_record(0) - self.assertIsInstance(log_record.body, str) + self.assertFalse(my_object.__str__.called) + self.assertFalse(my_object.__int__.called) +@contextmanager def set_up_test_logging(level, formatter=None): logger_provider = LoggerProvider() processor = FakeProcessor() @@ -262,7 +269,8 @@ def set_up_test_logging(level, formatter=None): if formatter: handler.setFormatter(formatter) logger.addHandler(handler) - return processor, logger + yield processor, logger + logger.removeHandler(handler) class FakeProcessor(LogRecordProcessor): From b9f0de9a93cb8185e0e8e7d511d7e5e8c026d5f1 Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Wed, 28 Aug 2024 11:37:34 +0200 Subject: [PATCH 11/15] fix: remove attribute extension to keep this PR on the enabling of formatting only --- .../sdk/_logs/_internal/__init__.py | 67 +++++++++---------- 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index 75c68767b56..f1f9c42fbc9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -56,35 +56,6 @@ _DEFAULT_OTEL_ATTRIBUTE_COUNT_LIMIT = 128 _ENV_VALUE_UNSET = "" -_EXCLUDED_ATTRIBUTES = { - # pseudo-private log-record attributes, they should always be dropped - "args", - "msg", - "message", - "stack_info", - "exc_info", - "exc_text", - # attributes that are retained, but with a different name - # following semantic conventions - "pathname", - "funcName", - "lineno", - "thread", - "threadName", - # attributes that are omitted because no semantic convention exists for them yet - "asctime", - "created", - "filename", - "levelname", - "levelno", - "module", - "msecs", - "name", - "process", - "processName", - "relativeCreated", - "taskName", -} class LogDroppedAttributesWarning(UserWarning): @@ -456,6 +427,37 @@ def force_flush(self, timeout_millis: int = 30000) -> bool: return True +# skip natural LogRecord attributes +# http://docs.python.org/library/logging.html#logrecord-attributes +_RESERVED_ATTRS = frozenset( + ( + "asctime", + "args", + "created", + "exc_info", + "exc_text", + "filename", + "funcName", + "message", + "levelname", + "levelno", + "lineno", + "module", + "msecs", + "msg", + "name", + "pathname", + "process", + "processName", + "relativeCreated", + "stack_info", + "thread", + "threadName", + "taskName", + ) +) + + class LoggingHandler(logging.Handler): """A handler class which writes logging records, in OTLP format, to a network destination or file. Supports signals from the `logging` module. @@ -476,18 +478,13 @@ def __init__( @staticmethod def _get_attributes(record: logging.LogRecord) -> Attributes: attributes = { - k: v - for k, v in vars(record).items() - if k not in _EXCLUDED_ATTRIBUTES and v is not None + k: v for k, v in vars(record).items() if k not in _RESERVED_ATTRS } # Add standard code attributes for logs. attributes[SpanAttributes.CODE_FILEPATH] = record.pathname attributes[SpanAttributes.CODE_FUNCTION] = record.funcName attributes[SpanAttributes.CODE_LINENO] = record.lineno - # Add thread identifiers for logs. - attributes[SpanAttributes.THREAD_ID] = record.thread - attributes[SpanAttributes.THREAD_NAME] = record.threadName if record.exc_info: exctype, value, tb = record.exc_info From aace0cbfa6f705e84c393917ce7e6d4a1029097a Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Wed, 28 Aug 2024 12:07:34 +0200 Subject: [PATCH 12/15] fix: add one more test to ensure what default formatting looks like --- opentelemetry-sdk/tests/logs/test_handler.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 10eb12d3d2c..3fda95589e6 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -224,7 +224,16 @@ def test_log_record_args_are_translated(self): }, ) - def test_format_is_called(self): + def test_default_format_is_called(self): + with set_up_test_logging(logging.WARNING) as (processor, logger): + logger.warning("Test message") + + log_record = processor.get_log_record(0) + self.assertEqual( + log_record.body, "Test message" + ) + + def test_custom_format_is_called(self): with set_up_test_logging(logging.WARNING, formatter=logging.Formatter("%(name)s - %(levelname)s - %(message)s")) as (processor, logger): logger.warning("Test message") From 4a2a9840d97b706df0db2ee119bac7da45c3a08a Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Wed, 28 Aug 2024 12:15:24 +0200 Subject: [PATCH 13/15] chore: ran black --- opentelemetry-sdk/tests/logs/test_handler.py | 47 +++++++++++++------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 3fda95589e6..bc21d4f6dbb 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -100,7 +100,9 @@ def test_log_record_no_span_context(self): log_record = processor.get_log_record(0) self.assertIsNotNone(log_record) - self.assertEqual(log_record.trace_id, INVALID_SPAN_CONTEXT.trace_id) + self.assertEqual( + log_record.trace_id, INVALID_SPAN_CONTEXT.trace_id + ) self.assertEqual(log_record.span_id, INVALID_SPAN_CONTEXT.span_id) self.assertEqual( log_record.trace_flags, INVALID_SPAN_CONTEXT.trace_flags @@ -119,7 +121,9 @@ def test_log_record_user_attributes(self): with set_up_test_logging(logging.WARNING) as (processor, logger): # Assert emit gets called for warning message with self.assertLogs(level=logging.WARNING): - logger.warning("Warning message", extra={"http.status_code": 200}) + logger.warning( + "Warning message", extra={"http.status_code": 200} + ) log_record = processor.get_log_record(0) @@ -139,8 +143,12 @@ def test_log_record_user_attributes(self): ) # The line of the log statement is not a constant (changing tests may change that), # so only check that the attribute is present. - self.assertTrue(SpanAttributes.CODE_LINENO in log_record.attributes) - self.assertTrue(isinstance(log_record.attributes, BoundedAttributes)) + self.assertTrue( + SpanAttributes.CODE_LINENO in log_record.attributes + ) + self.assertTrue( + isinstance(log_record.attributes, BoundedAttributes) + ) def test_log_record_exception(self): """Exception information will be included in attributes""" @@ -185,7 +193,9 @@ def test_log_exc_info_false(self): self.assertIsNotNone(log_record) self.assertEqual(log_record.body, "Zero Division Error") - self.assertNotIn(SpanAttributes.EXCEPTION_TYPE, log_record.attributes) + self.assertNotIn( + SpanAttributes.EXCEPTION_TYPE, log_record.attributes + ) self.assertNotIn( SpanAttributes.EXCEPTION_MESSAGE, log_record.attributes ) @@ -202,13 +212,19 @@ def test_log_record_trace_correlation(self): log_record = processor.get_log_record(0) - self.assertEqual(log_record.body, "Critical message within span") + self.assertEqual( + log_record.body, "Critical message within span" + ) self.assertEqual(log_record.severity_text, "CRITICAL") - self.assertEqual(log_record.severity_number, SeverityNumber.FATAL) + self.assertEqual( + log_record.severity_number, SeverityNumber.FATAL + ) span_context = span.get_span_context() self.assertEqual(log_record.trace_id, span_context.trace_id) self.assertEqual(log_record.span_id, span_context.span_id) - self.assertEqual(log_record.trace_flags, span_context.trace_flags) + self.assertEqual( + log_record.trace_flags, span_context.trace_flags + ) def test_log_record_args_are_translated(self): with set_up_test_logging(logging.WARNING) as (processor, logger): @@ -229,18 +245,19 @@ def test_default_format_is_called(self): logger.warning("Test message") log_record = processor.get_log_record(0) - self.assertEqual( - log_record.body, "Test message" - ) + self.assertEqual(log_record.body, "Test message") def test_custom_format_is_called(self): - with set_up_test_logging(logging.WARNING, formatter=logging.Formatter("%(name)s - %(levelname)s - %(message)s")) as (processor, logger): + with set_up_test_logging( + logging.WARNING, + formatter=logging.Formatter( + "%(name)s - %(levelname)s - %(message)s" + ), + ) as (processor, logger): logger.warning("Test message") log_record = processor.get_log_record(0) - self.assertEqual( - log_record.body, "foo - WARNING - Test message" - ) + self.assertEqual(log_record.body, "foo - WARNING - Test message") def test_log_body_is_always_string(self): with set_up_test_logging(logging.WARNING) as (processor, logger): From d731803107212127381b23b509548365c79774c9 Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Wed, 28 Aug 2024 13:21:41 +0200 Subject: [PATCH 14/15] fix: added checking the actual log message to percent-format test --- opentelemetry-sdk/tests/logs/test_handler.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index bc21d4f6dbb..5f6a7eb33d8 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -267,20 +267,24 @@ def test_log_body_is_always_string(self): self.assertIsInstance(log_record.body, str) def test_args_get_transformed_if_logged(self): - with set_up_test_logging(logging.WARNING) as (_, logger): + with set_up_test_logging(logging.WARNING) as (processor, logger): my_object = MagicMock() + my_object.__str__.return_value = "foo" + my_object.__int__.return_value = 42 logger.warning("%s - %d", my_object, my_object) + self.assertEqual(processor.get_log_record(0).body, "foo - 42") self.assertTrue(my_object.__str__.called) self.assertTrue(my_object.__int__.called) def test_args_do_not_get_transformed_if_not_logged(self): - with set_up_test_logging(logging.WARNING) as (_, logger): + with set_up_test_logging(logging.WARNING) as (processor, logger): my_object = MagicMock() logger.info("%s - %d", my_object, my_object) + self.assertEqual(processor.log_data_emitted, []) self.assertFalse(my_object.__str__.called) self.assertFalse(my_object.__int__.called) From 23eb08ba40de5c65f493a71bff50a4c2c5d10f19 Mon Sep 17 00:00:00 2001 From: Arne Caratti Date: Wed, 28 Aug 2024 13:39:46 +0200 Subject: [PATCH 15/15] fix: fixed unit test to use the context-manager setup for the logging setup --- opentelemetry-sdk/tests/logs/test_handler.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index a10b078e920..119d18c4e68 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -182,20 +182,20 @@ def test_log_record_exception(self): def test_log_record_recursive_exception(self): """Exception information will be included in attributes even though it is recursive""" - processor, logger = set_up_test_logging(logging.ERROR) + with set_up_test_logging(logging.ERROR) as (processor, logger): - try: - raise ZeroDivisionError( - ZeroDivisionError(ZeroDivisionError("division by zero")) - ) - except ZeroDivisionError: - with self.assertLogs(level=logging.ERROR): - logger.exception("Zero Division Error") + try: + raise ZeroDivisionError( + ZeroDivisionError(ZeroDivisionError("division by zero")) + ) + except ZeroDivisionError: + with self.assertLogs(level=logging.ERROR): + logger.exception("Zero Division Error") log_record = processor.get_log_record(0) self.assertIsNotNone(log_record) - self.assertEqual(log_record.body, "Zero Division Error") + self.assertIn("Zero Division Error", log_record.body) self.assertEqual( log_record.attributes[SpanAttributes.EXCEPTION_TYPE], ZeroDivisionError.__name__,