From db5efc4f5931e05bfbe4bfc90598d875bb2ea31c Mon Sep 17 00:00:00 2001 From: Lubos Mjachky Date: Thu, 31 Oct 2024 18:10:04 +0100 Subject: [PATCH] Remove functional tests for opentelemetry instrumentation (cherry picked from commit 981af2ef7b01d19b6b40b2c3c1a5dcbbc0bdf7a7) --- .github/workflows/scripts/install.sh | 2 +- .../workflows/scripts/pre_before_script.sh | 4 +- .gitignore | 1 + .../api/test_telemetry_collection.py | 91 ----------- pulpcore/pytest_plugin.py | 62 ------- pulpcore/tests/functional/api/test_status.py | 29 +--- pulpcore/tests/functional/api/test_tasking.py | 25 +-- .../tests/functional/assets/otel_server.py | 152 ------------------ template_config.yml | 8 +- 9 files changed, 8 insertions(+), 366 deletions(-) delete mode 100644 pulp_file/tests/functional/api/test_telemetry_collection.py delete mode 100644 pulpcore/tests/functional/assets/otel_server.py diff --git a/.github/workflows/scripts/install.sh b/.github/workflows/scripts/install.sh index a08a4e940b..e0b3468e0a 100755 --- a/.github/workflows/scripts/install.sh +++ b/.github/workflows/scripts/install.sh @@ -120,7 +120,7 @@ if [ "$TEST" = "azure" ]; then command: "azurite-blob --blobHost 0.0.0.0"' vars/main.yaml sed -i -e '$a azure_test: true\ pulp_scenario_settings: {"api_root_rewrite_header": "X-API-Root", "domain_enabled": true}\ -pulp_scenario_env: {"otel_bsp_max_export_batch_size": 1, "otel_bsp_max_queue_size": 1, "otel_exporter_otlp_endpoint": "http://localhost:4318", "otel_exporter_otlp_protocol": "http/protobuf", "otel_metric_export_interval": 800, "pulp_otel_enabled": "true"}\ +pulp_scenario_env: {}\ ' vars/main.yaml fi diff --git a/.github/workflows/scripts/pre_before_script.sh b/.github/workflows/scripts/pre_before_script.sh index 0c1f8cb2cc..8abf8bd1a4 100644 --- a/.github/workflows/scripts/pre_before_script.sh +++ b/.github/workflows/scripts/pre_before_script.sh @@ -4,6 +4,6 @@ if [ "$TEST" = "azure" ]; then cmd_stdin_prefix bash -c "cat > /etc/nginx/pulp/api_root_rewrite.conf" < pulpcore/tests/functional/assets/api_root_rewrite.conf cmd_prefix bash -c "s6-rc -d change nginx" cmd_prefix bash -c "s6-rc -u change nginx" - cmd_stdin_prefix bash -c "cat > /var/lib/pulp/scripts/otel_server.py" < pulpcore/tests/functional/assets/otel_server.py - cmd_user_prefix nohup python3 /var/lib/pulp/scripts/otel_server.py & + # it is necessary to wait for a short period until the service starts; otherwise, any subsequent requests to Pulp's API endpoints will fail + sleep 1 fi diff --git a/.gitignore b/.gitignore index 2d068ce3f3..01492e3be0 100644 --- a/.gitignore +++ b/.gitignore @@ -41,6 +41,7 @@ tags # Vagrant .vagrant Vagrantfile +__pycache__/ # Built documentation docs/_build diff --git a/pulp_file/tests/functional/api/test_telemetry_collection.py b/pulp_file/tests/functional/api/test_telemetry_collection.py deleted file mode 100644 index 19b3bf4c24..0000000000 --- a/pulp_file/tests/functional/api/test_telemetry_collection.py +++ /dev/null @@ -1,91 +0,0 @@ -import requests -import uuid - -from urllib.parse import urljoin, urlparse -from django.conf import settings - -from pulpcore.client.pulp_file import FileFileDistribution, RepositoryAddRemoveContent - - -def test_get_requests( - file_bindings, - file_repo_with_auto_publish, - file_content_unit_with_name_factory, - gen_object_with_cleanup, - monitor_task, - received_otel_span, - test_path, -): - """Test if content-app correctly returns mime-types based on filenames.""" - content_units = [ - file_content_unit_with_name_factory("otel_test_file1.tar.gz"), - file_content_unit_with_name_factory("otel_test_file2.xml.gz"), - file_content_unit_with_name_factory("otel_test_file3.txt"), - ] - units_to_add = list(map(lambda f: f.pulp_href, content_units)) - data = RepositoryAddRemoveContent(add_content_units=units_to_add) - monitor_task( - file_bindings.RepositoriesFileApi.modify(file_repo_with_auto_publish.pulp_href, data).task - ) - - data = FileFileDistribution( - name=str(uuid.uuid4()), - base_path=str(uuid.uuid4()), - repository=file_repo_with_auto_publish.pulp_href, - ) - distribution = gen_object_with_cleanup(file_bindings.DistributionsFileApi, data) - - for content_unit in content_units: - url = urljoin(distribution.base_url, content_unit.relative_path) - content_path = urlparse(url).path - - s = requests.Session() - s.headers = {"User-Agent": test_path} - - if ( - settings.DEFAULT_FILE_STORAGE != "pulpcore.app.models.storage.FileSystem" - and settings.REDIRECT_TO_OBJECT_STORAGE - ): - status_code = 302 - else: - status_code = 200 - - s.get(url, allow_redirects=False) - assert received_otel_span( - { - "http.method": "GET", - "http.target": content_path, - "http.status_code": status_code, - "http.user_agent": test_path, - } - ) - - s.get(url + "fail") - assert received_otel_span( - { - "http.method": "GET", - "http.target": content_path + "fail", - "http.status_code": 404, - "http.user_agent": test_path, - } - ) - - s.post(url, data={}) - assert received_otel_span( - { - "http.method": "POST", - "http.target": content_path, - "http.status_code": 405, - "http.user_agent": test_path, - } - ) - - s.head(url, allow_redirects=False) - assert received_otel_span( - { - "http.method": "HEAD", - "http.target": content_path, - "http.status_code": status_code, - "http.user_agent": test_path, - } - ) diff --git a/pulpcore/pytest_plugin.py b/pulpcore/pytest_plugin.py index 16b07822ee..b43b3cd14e 100644 --- a/pulpcore/pytest_plugin.py +++ b/pulpcore/pytest_plugin.py @@ -209,68 +209,6 @@ def make_url(self, path): return f"{protocol_handler}{self.host}:{self.port}{path}" -@pytest.fixture(scope="session") -def received_otel_span(): - """A fixture for checking the presence of specific spans on the otel collector server. - - Ensure the collector server is up and running before executing tests with this fixture. To do - so, please, run the server as follows: python3 pulpcore/tests/functional/assets/otel_server.py - """ - - def _received_otel_span(data, retries=3): - if os.environ.get("PULP_OTEL_ENABLED") != "true": - # pretend everything is working as expected if tests are run from - # a non-configured runner - return True - - async def _send_request(): - async with aiohttp.ClientSession(raise_for_status=False) as session: - otel_server_url = os.environ.get("OTEL_EXPORTER_OTLP_ENDPOINT") - async with session.post(f"{otel_server_url}/test", json=data) as response: - return response.status - - while retries: - status = asyncio.run(_send_request()) - if status == 200: - return True - sleep(2) - retries -= 1 - return False - - return _received_otel_span - - -@pytest.fixture(scope="session") -def received_otel_metrics(): - """A fixture for checking the presence of specific metrics on the otel collector server. - - Ensure the collector server is up and running before executing tests with this fixture. To do - so, please, run the server as follows: python3 pulpcore/tests/functional/assets/otel_server.py - """ - - def _received_otel_metric(data, retries=3): - if os.environ.get("PULP_OTEL_ENABLED") != "true": - # pretend everything is working as expected if tests are run from - # a non-configured runner - return True - - async def _send_request(): - async with aiohttp.ClientSession(raise_for_status=False) as session: - otel_server_url = os.environ.get("OTEL_EXPORTER_OTLP_ENDPOINT") - async with session.post(f"{otel_server_url}/metrics_test", json=data) as response: - return response.status - - while retries: - status = asyncio.run(_send_request()) - if status == 200: - return True - sleep(2) - retries -= 1 - return False - - return _received_otel_metric - - @pytest.fixture def test_path(): return os.getenv("PYTEST_CURRENT_TEST").split()[0] diff --git a/pulpcore/tests/functional/api/test_status.py b/pulpcore/tests/functional/api/test_status.py index 0bb9e348b9..861b431b95 100644 --- a/pulpcore/tests/functional/api/test_status.py +++ b/pulpcore/tests/functional/api/test_status.py @@ -59,25 +59,17 @@ @pytest.mark.parallel -def test_get_authenticated(test_path, pulpcore_bindings, received_otel_span): +def test_get_authenticated(test_path, pulpcore_bindings): """GET the status path with valid credentials. Verify the response with :meth:`verify_get_response`. """ response = pulpcore_bindings.StatusApi.status_read() verify_get_response(response.to_dict(), STATUS) - assert received_otel_span( - { - "http.method": "GET", - "http.target": "/pulp/api/v3/status/", - "http.status_code": 200, - "http.user_agent": test_path, - } - ) @pytest.mark.parallel -def test_get_unauthenticated(test_path, pulpcore_bindings, anonymous_user, received_otel_span): +def test_get_unauthenticated(test_path, pulpcore_bindings, anonymous_user): """GET the status path with no credentials. Verify the response with :meth:`verify_get_response`. @@ -85,14 +77,6 @@ def test_get_unauthenticated(test_path, pulpcore_bindings, anonymous_user, recei with anonymous_user: response = pulpcore_bindings.StatusApi.status_read() verify_get_response(response.to_dict(), STATUS) - assert received_otel_span( - { - "http.method": "GET", - "http.target": "/pulp/api/v3/status/", - "http.status_code": 200, - "http.user_agent": test_path, - } - ) @pytest.mark.parallel @@ -101,7 +85,6 @@ def test_post_authenticated( pulp_api_v3_path, pulp_api_v3_url, pulpcore_bindings, - received_otel_span, ): """POST the status path with valid credentials. @@ -117,14 +100,6 @@ def test_post_authenticated( pulpcore_bindings.client.request("POST", status_url, headers={"User-Agent": test_path}) assert e.value.status == 405 - assert received_otel_span( - { - "http.method": "POST", - "http.target": f"{pulp_api_v3_path}status/", - "http.status_code": 405, - "http.user_agent": test_path, - } - ) @pytest.mark.parallel diff --git a/pulpcore/tests/functional/api/test_tasking.py b/pulpcore/tests/functional/api/test_tasking.py index af090b05bf..06584d5ec2 100644 --- a/pulpcore/tests/functional/api/test_tasking.py +++ b/pulpcore/tests/functional/api/test_tasking.py @@ -1,6 +1,5 @@ """Tests related to the tasking system.""" -import os import json import pytest import time @@ -335,12 +334,7 @@ def test_task_version_prevent_pickup(dispatch_task, pulpcore_bindings): pulpcore_bindings.TasksApi.tasks_cancel(task_href, {"state": "canceled"}) -def test_emmiting_unblocked_task_telemetry( - dispatch_task, pulpcore_bindings, pulp_settings, received_otel_metrics -): - if os.getenv("PULP_OTEL_ENABLED").lower() != "true": - pytest.skip("Need PULP_OTEL_ENABLED to run this test.") - +def test_emmiting_unblocked_task_telemetry(dispatch_task, pulpcore_bindings, pulp_settings): # Checking online workers ready to get a task workers_online = pulpcore_bindings.WorkersApi.list(online="true").count @@ -356,23 +350,6 @@ def test_emmiting_unblocked_task_telemetry( task = pulpcore_bindings.TasksApi.read(task_href) assert task.state == "waiting" - # And trigger the metrics - assert received_otel_metrics( - { - "name": "tasks_unblocked_queue", - "description": "Number of unblocked tasks waiting in the queue.", - "unit": "tasks", - } - ) - - assert received_otel_metrics( - { - "name": "tasks_longest_unblocked_time", - "description": "The age of the longest waiting task.", - "unit": "seconds", - } - ) - [ pulpcore_bindings.TasksApi.tasks_cancel(task_href, {"state": "canceled"}) for task_href in resident_task_hrefs diff --git a/pulpcore/tests/functional/assets/otel_server.py b/pulpcore/tests/functional/assets/otel_server.py deleted file mode 100644 index c9acf36a71..0000000000 --- a/pulpcore/tests/functional/assets/otel_server.py +++ /dev/null @@ -1,152 +0,0 @@ -import asyncio -import json -import logging -import os -import sys -import threading - -from aiohttp import web - -from opentelemetry.proto.trace.v1.trace_pb2 import TracesData -from opentelemetry.proto.metrics.v1.metrics_pb2 import MetricsData - -_logger = logging.getLogger(__name__) - - -class ThreadedAiohttpServer(threading.Thread): - def __init__(self, app, host, port, ssl_ctx): - super().__init__() - self.app = app - self.host = host - self.port = port - self.ssl_ctx = ssl_ctx - self.loop = asyncio.new_event_loop() - - async def arun(self): - runner = web.AppRunner(self.app) - await runner.setup() - site = web.TCPSite(runner, host=self.host, port=self.port, ssl_context=self.ssl_ctx) - await site.start() - async with self.shutdown_condition: - await self.shutdown_condition.wait() - await runner.cleanup() - - def run(self): - asyncio.set_event_loop(self.loop) - self.shutdown_condition = asyncio.Condition() - self.loop.run_until_complete(self.arun()) - - async def astop(self): - async with self.shutdown_condition: - self.shutdown_condition.notify_all() - - def stop(self): - asyncio.run_coroutine_threadsafe(self.astop(), self.loop) - - -def _otel_collector(): - if ( - os.environ.get("PULP_OTEL_ENABLED") != "true" - or os.environ.get("OTEL_EXPORTER_OTLP_ENDPOINT") != "http://localhost:4318" - or os.environ.get("OTEL_EXPORTER_OTLP_PROTOCOL") != "http/protobuf" - ): - _logger.info("Telemetry was not configured. Exiting...") - sys.exit(0) - else: - _logger.info("Booting up the otel collector server...") - - spans = [] - metrics = [] - - async def _null_handler(request): - raise web.HTTPOk() - - async def _traces_handler(request): - traces_data = TracesData() - traces_data.ParseFromString(await request.read()) - for resource_span in traces_data.resource_spans: - for scope_span in resource_span.scope_spans: - for span in scope_span.spans: - attrs = { - item.key: getattr(item.value, item.value.WhichOneof("value")) - for item in span.attributes - } - spans.append(attrs) - raise web.HTTPOk() - - async def _metrics_handler(request): - disabled_metrics = {"http.server.active_requests"} - - metrics_data = MetricsData() - metrics_data.ParseFromString(await request.read()) - for resource_metric in metrics_data.resource_metrics: - for scope_metric in resource_metric.scope_metrics: - for metric in scope_metric.metrics: - if metric.name in disabled_metrics: - _logger.info("Dropping {} metric".format(metric.name)) - break - translated_metric = {} - translated_metric["name"] = metric.name - translated_metric["description"] = metric.description - translated_metric["unit"] = metric.unit - metrics.append(translated_metric) - _logger.info("Received a {} metric meter".format(translated_metric["name"])) - raise web.HTTPOk() - - async def _test_handler(request): - try: - attrs = await request.json() - except json.decoder.JSONDecodeError: - raise web.HTTPNotFound() - - matched_span = next( - (span for span in spans if all((span.get(k) == v for k, v in attrs.items()))), - None, - ) - if matched_span: - raise web.HTTPOk() - else: - raise web.HTTPNotFound() - - async def _metrics_test_handler(request): - try: - attrs = await request.json() - except json.decoder.JSONDecodeError: - raise web.HTTPNotFound() - - matched_metric = next( - ( - metric - for metric in metrics - if all((metric.get(key) == value for key, value in attrs.items())) - ), - None, - ) - if matched_metric: - metrics.remove(matched_metric) - raise web.HTTPOk() - else: - raise web.HTTPNotFound() - - async def _read_handler(request): - return web.Response(text=json.dumps(metrics)) - - app = web.Application() - app.add_routes( - [ - web.post("/v1/metrics", _metrics_handler), - web.post("/v1/traces", _traces_handler), - web.post("/test", _test_handler), - web.post("/metrics_test", _metrics_test_handler), - web.get("/read", _read_handler), - ] - ) - - host = "127.0.0.1" - port = 4318 - collector_server = ThreadedAiohttpServer(app, host, port, ssl_ctx=None) - collector_server.start() - - -if __name__ == "__main__": - _otel_collector() diff --git a/template_config.yml b/template_config.yml index f876ea6d74..ab6595ea7a 100644 --- a/template_config.yml +++ b/template_config.yml @@ -43,13 +43,7 @@ post_job_template: null pre_job_template: null pulp_env: PULP_CA_BUNDLE: /etc/pulp/certs/pulp_webserver.crt -pulp_env_azure: - otel_bsp_max_export_batch_size: 1 - otel_bsp_max_queue_size: 1 - otel_exporter_otlp_endpoint: http://localhost:4318 - otel_exporter_otlp_protocol: http/protobuf - otel_metric_export_interval: 800 - pulp_otel_enabled: 'true' +pulp_env_azure: {} pulp_env_gcp: {} pulp_env_s3: {} pulp_scheme: https