From aed50b793c244adfba55d8fb5dad52d60ef94ab7 Mon Sep 17 00:00:00 2001 From: Andrei Matveyeu Date: Fri, 3 May 2024 07:35:02 +0200 Subject: [PATCH] Code review changes --- projects/etos_suite_runner/requirements.txt | 4 ++++ .../src/etos_suite_runner/__init__.py | 7 ++++--- .../etos_suite_runner/src/etos_suite_runner/esr.py | 11 +++++++---- .../src/etos_suite_runner/lib/executor.py | 7 +++---- .../src/etos_suite_runner/lib/otel_tracing.py | 8 +++++--- .../src/etos_suite_runner/lib/runner.py | 6 +++--- .../src/etos_suite_runner/lib/suite.py | 10 ++++++---- 7 files changed, 32 insertions(+), 21 deletions(-) diff --git a/projects/etos_suite_runner/requirements.txt b/projects/etos_suite_runner/requirements.txt index d3712cb..3ef02bf 100644 --- a/projects/etos_suite_runner/requirements.txt +++ b/projects/etos_suite_runner/requirements.txt @@ -19,10 +19,14 @@ PyScaffold==3.2.3 packageurl-python~=0.11 cryptography>=42.0.4,<43.0.0 <<<<<<< HEAD +<<<<<<< HEAD etos_lib==4.0.0 etos_environment_provider~=4.1 ======= etos_lib==4.1.1 +======= +etos_lib==4.2.0 +>>>>>>> 11b187c (Code review changes) etos_environment_provider~=4.2 opentelemetry-api~=1.21 opentelemetry-exporter-otlp~=1.21 diff --git a/projects/etos_suite_runner/src/etos_suite_runner/__init__.py b/projects/etos_suite_runner/src/etos_suite_runner/__init__.py index cea339f..c921c33 100644 --- a/projects/etos_suite_runner/src/etos_suite_runner/__init__.py +++ b/projects/etos_suite_runner/src/etos_suite_runner/__init__.py @@ -47,9 +47,10 @@ if os.getenv("OTEL_COLLECTOR_HOST"): os.environ["OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"] = os.getenv("OTEL_COLLECTOR_HOST") else: - LOGGER.debug("Environment variable OTEL_EXPORTER_OTLP_TRACES_ENDPOINT not used.") - LOGGER.debug("To specify an OpenTelemetry collector host use OTEL_COLLECTOR_HOST.") - del os.environ["OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"] + if "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT" in os.environ: + LOGGER.debug("Environment variable OTEL_EXPORTER_OTLP_TRACES_ENDPOINT not used.") + LOGGER.debug("To specify an OpenTelemetry collector host use OTEL_COLLECTOR_HOST.") + del os.environ["OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"] if os.getenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"): LOGGER.info( diff --git a/projects/etos_suite_runner/src/etos_suite_runner/esr.py b/projects/etos_suite_runner/src/etos_suite_runner/esr.py index ea5f5e2..543dd40 100644 --- a/projects/etos_suite_runner/src/etos_suite_runner/esr.py +++ b/projects/etos_suite_runner/src/etos_suite_runner/esr.py @@ -27,7 +27,6 @@ from environment_provider.environment import release_full_environment from etos_lib import ETOS from etos_lib.logging.logger import FORMAT_CONFIG -from etos_lib.opentelemetry.semconv import Attributes as SemConvAttributes from jsontas.jsontas import JsonTas import opentelemetry @@ -74,8 +73,10 @@ def _request_environment(self, ids: list[str]) -> None: span_name = "request_environment" suite_context = get_current_context() with self.otel_tracer.start_as_current_span( - span_name, context=suite_context, kind=opentelemetry.trace.SpanKind.CLIENT, - ) as span: + span_name, + context=suite_context, + kind=opentelemetry.trace.SpanKind.CLIENT, + ): try: provider = EnvironmentProvider(self.params.tercc.meta.event_id, ids, copy=False) result = provider.run() @@ -111,7 +112,9 @@ def _release_environment(self) -> None: span_name = "release_full_environment" suite_context = get_current_context() with self.otel_tracer.start_as_current_span( - span_name, context=suite_context, kind=opentelemetry.trace.SpanKind.CLIENT, + span_name, + context=suite_context, + kind=opentelemetry.trace.SpanKind.CLIENT, ): status, message = release_full_environment( etos=self.etos, jsontas=jsontas, suite_id=self.params.tercc.meta.event_id diff --git a/projects/etos_suite_runner/src/etos_suite_runner/lib/executor.py b/projects/etos_suite_runner/src/etos_suite_runner/lib/executor.py index aad45a0..41743ba 100644 --- a/projects/etos_suite_runner/src/etos_suite_runner/lib/executor.py +++ b/projects/etos_suite_runner/src/etos_suite_runner/lib/executor.py @@ -29,6 +29,7 @@ from .otel_tracing import OpenTelemetryBase + class TestStartException(Exception): """Exception when starting tests.""" @@ -94,11 +95,9 @@ def run_tests(self, test_suite: dict) -> None: request["auth"] = self.__auth(**request["auth"]) method = getattr(self.etos.http, request.pop("method").lower()) span_name = "start_execution_space" - with self.tracer.start_as_current_span( - span_name, kind=trace.SpanKind.CLIENT - ) as span: + with self.tracer.start_as_current_span(span_name, kind=trace.SpanKind.CLIENT) as span: span.set_attribute(SemConvAttributes.EXECUTOR_ID, executor["id"]) - span.set_attribute("http.request.body", dumps(request, indent=4)) + span.set_attribute("http.request.body", dumps(request)) try: response = method(**request) response.raise_for_status() diff --git a/projects/etos_suite_runner/src/etos_suite_runner/lib/otel_tracing.py b/projects/etos_suite_runner/src/etos_suite_runner/lib/otel_tracing.py index ee596fe..76e9f63 100644 --- a/projects/etos_suite_runner/src/etos_suite_runner/lib/otel_tracing.py +++ b/projects/etos_suite_runner/src/etos_suite_runner/lib/otel_tracing.py @@ -30,6 +30,7 @@ class OpenTelemetryBase: """Base functionality for OpenTelemetry data collection.""" + # pylint: disable=too-few-public-methods @staticmethod def _record_exception(exc) -> None: """Record the given exception to the current OpenTelemetry span.""" @@ -46,9 +47,9 @@ def get(self, carrier, key): """Get value using the given carrier variable and key.""" value = os.getenv(carrier) if value is not None: - pairs = value.split(',') + pairs = value.split(",") for pair in pairs: - k, v = pair.split('=', 1) + k, v = pair.split("=", 1) if k == key: return [v] return [] @@ -57,9 +58,10 @@ def keys(self, carrier): """Get keys of the given carrier variable.""" value = os.getenv(carrier) if value is not None: - return [pair.split('=')[0] for pair in value.split(',') if '=' in pair] + return [pair.split("=")[0] for pair in value.split(",") if "=" in pair] return [] + def get_current_context() -> opentelemetry.context.context.Context: """Get current context propagated via environment variable OTEL_CONTEXT.""" propagator = TraceContextTextMapPropagator() diff --git a/projects/etos_suite_runner/src/etos_suite_runner/lib/runner.py b/projects/etos_suite_runner/src/etos_suite_runner/lib/runner.py index 7ef2eb2..7624aaa 100644 --- a/projects/etos_suite_runner/src/etos_suite_runner/lib/runner.py +++ b/projects/etos_suite_runner/src/etos_suite_runner/lib/runner.py @@ -17,8 +17,6 @@ import logging from multiprocessing.pool import ThreadPool -import opentelemetry.trace - from environment_provider.environment import release_full_environment from etos_lib.logging.logger import FORMAT_CONFIG from jsontas.jsontas import JsonTas @@ -59,7 +57,9 @@ def _release_environment(self): jsontas = JsonTas() span_name = "release_full_environment" with self.otel_tracer.start_as_current_span( - span_name, context=self.otel_suite_context, kind=opentelemetry.trace.SpanKind.CLIENT, + span_name, + context=self.otel_suite_context, + kind=opentelemetry.trace.SpanKind.CLIENT, ): status, message = release_full_environment( etos=self.etos, jsontas=jsontas, suite_id=self.params.tercc.meta.event_id diff --git a/projects/etos_suite_runner/src/etos_suite_runner/lib/suite.py b/projects/etos_suite_runner/src/etos_suite_runner/lib/suite.py index d3b840f..9a3284a 100644 --- a/projects/etos_suite_runner/src/etos_suite_runner/lib/suite.py +++ b/projects/etos_suite_runner/src/etos_suite_runner/lib/suite.py @@ -21,7 +21,6 @@ from typing import Iterator from eiffellib.events import EiffelTestSuiteStartedEvent -import opentelemetry.trace from environment_provider.lib.registry import ProviderRegistry from environment_provider.environment import release_environment from etos_lib import ETOS @@ -108,7 +107,9 @@ def start(self, identifier: str, otel_context: opentelemetry.context.context.Con # This is because the subsuite is running in a separate thread. span_name = "execute_testrunner" with self.otel_tracer.start_as_current_span( - span_name, context=otel_context, kind=opentelemetry.trace.SpanKind.CLIENT, + span_name, + context=otel_context, + kind=opentelemetry.trace.SpanKind.CLIENT, ) as span: span.set_attribute(SemConvAttributes.SUBSUITE_ID, identifier) FORMAT_CONFIG.identifier = identifier @@ -153,7 +154,8 @@ def release(self, testrun_id) -> None: span_name = "release_environment" with self.otel_tracer.start_as_current_span( - span_name, kind=opentelemetry.trace.SpanKind.CLIENT, + span_name, + kind=opentelemetry.trace.SpanKind.CLIENT, ) as span: failure = release_environment( etos=self.etos, @@ -162,7 +164,7 @@ def release(self, testrun_id) -> None: sub_suite=self.environment, ) span.set_attribute(SemConvAttributes.TESTRUN_ID, testrun_id) - span.set_attribute(SemConvAttributes.ENVIRONMENT, json.dumps(self.environment, indent=4)) + span.set_attribute(SemConvAttributes.ENVIRONMENT, json.dumps(self.environment)) if failure is not None: self.logger.exception( "Failed to check in %r", self.environment["id"], extra={"user_log": True}