🐛 Fix aiohttp server autoinstrumentation (#6391)

Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Dustin Kaiser <[email protected]> Co-authored-by: Andrei Neagu <[email protected]> Co-authored-by: Andrei Neagu <[email protected]> Co-authored-by: Odei Maiz <[email protected]> Co-authored-by: Sylvain <[email protected]> Co-authored-by: Pedro Crespo-Valero <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: matusdrobuliak66 <[email protected]> Co-authored-by: Julian Querido <[email protected]> Co-authored-by: Odei Maiz <[email protected]>
ITISFoundation · Oct 15, 2024 · 5dde4a4 · 5dde4a4
1 parent ad0a35d
commit 5dde4a4
Show file tree

Hide file tree

Showing 5 changed files with 206 additions and 149 deletions.
diff --git a/packages/service-library/src/servicelib/aiohttp/tracing.py b/packages/service-library/src/servicelib/aiohttp/tracing.py
@@ -12,8 +12,8 @@
 from opentelemetry.instrumentation.aiohttp_client import (  # pylint:disable=no-name-in-module
     AioHttpClientInstrumentor,
 )
-from opentelemetry.instrumentation.aiohttp_server import (  # pylint:disable=no-name-in-module
-    AioHttpServerInstrumentor,
+from opentelemetry.instrumentation.aiohttp_server import (
+    middleware as aiohttp_server_opentelemetry_middleware,  # pylint:disable=no-name-in-module
 )
 from opentelemetry.instrumentation.aiopg import (  # pylint:disable=no-name-in-module
     AiopgInstrumentor,
@@ -72,8 +72,24 @@ def setup_tracing(
 
     # Add the span processor to the tracer provider
     tracer_provider.add_span_processor(BatchSpanProcessor(otlp_exporter))  # type: ignore[attr-defined] # https://github.com/open-telemetry/opentelemetry-python/issues/3713
-    # Instrument aiohttp server and client
-    AioHttpServerInstrumentor().instrument()
+    # Instrument aiohttp server
+    # Explanation for custom middleware call DK 10/2024:
+    # OpenTelemetry Aiohttp autoinstrumentation is meant to be used by only calling `AioHttpServerInstrumentor().instrument()`
+    # The call `AioHttpServerInstrumentor().instrument()` monkeypatches the __init__() of aiohttp's web.application() to inject the tracing middleware, in it's `__init__()`.
+    # In simcore, we want to switch tracing on or off using the simcore-settings-library.
+    # The simcore-settings library in turn depends on the instance of web.application(), i.e. the aiohttp webserver, to exist. So here we face a hen-and-egg problem.
+    # At the time when the instrumentation should be configured, the instance of web.application already exists and the overwrite to the __init__() is never called
+    #
+    # Since the code that is provided (monkeypatched) in the __init__ that the opentelemetry-autoinstrumentation-library provides is only 4 lines,
+    # just adding a middleware, we are free to simply execute this "missed call" [since we can't call the monkeypatch'ed __init__()] in this following line:
+    app.middlewares.insert(0, aiohttp_server_opentelemetry_middleware)
+    # Code of the aiohttp server instrumentation: github.com/open-telemetry/opentelemetry-python-contrib/blob/eccb05c808a7d797ef5b6ecefed3590664426fbf/instrumentation/opentelemetry-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py#L246
+    # For reference, the above statement was written for:
+    # - osparc-simcore 1.77.x
+    # - opentelemetry-api==1.27.0
+    # - opentelemetry-instrumentation==0.48b0
+
+    # Instrument aiohttp client
     AioHttpClientInstrumentor().instrument()
     if instrument_aiopg:
         AiopgInstrumentor().instrument()

diff --git a/services/datcore-adapter/Dockerfile b/services/datcore-adapter/Dockerfile
@@ -4,7 +4,7 @@ FROM python:${PYTHON_VERSION}-slim-bookworm as base
 
 #
 #  USAGE:
-#     cd sercices/datcore-adapter
+#     cd services/datcore-adapter
 #     docker build -f Dockerfile -t datcore-adapter:prod --target production ../../
 #     docker run datcore-adapter:prod
 #

diff --git a/services/web/server/tests/unit/isolated/conftest.py b/services/web/server/tests/unit/isolated/conftest.py
@@ -7,7 +7,10 @@
 from faker import Faker
 from pytest_mock import MockerFixture
 from pytest_simcore.helpers.dict_tools import ConfigDict
-from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict
+from pytest_simcore.helpers.monkeypatch_envs import (
+    setenvs_from_dict,
+    setenvs_from_envfile,
+)
 from pytest_simcore.helpers.typing_env import EnvVarsDict
 
 
@@ -89,6 +92,145 @@ def mock_env_deployer_pipeline(monkeypatch: pytest.MonkeyPatch) -> EnvVarsDict:
     )
 
 
+@pytest.fixture
+def mock_env_devel_environment(
+    mock_env_devel_environment: EnvVarsDict,  # pylint: disable=redefined-outer-name
+    monkeypatch: pytest.MonkeyPatch,
+) -> EnvVarsDict:
+    # Overrides to ensure dev-features are enabled testings
+    return mock_env_devel_environment | setenvs_from_dict(
+        monkeypatch,
+        envs={
+            "WEBSERVER_DEV_FEATURES_ENABLED": "1",
+        },
+    )
+
+
+@pytest.fixture
+def mock_env_makefile(monkeypatch: pytest.MonkeyPatch) -> EnvVarsDict:
+    """envvars produced @Makefile (export)"""
+    # TODO: add Makefile recipe 'make dump-envs' to produce the file we load here
+    return setenvs_from_dict(
+        monkeypatch,
+        {
+            "API_SERVER_API_VERSION": "0.3.0",
+            "BUILD_DATE": "2022-01-14T21:28:15Z",
+            "CATALOG_API_VERSION": "0.3.2",
+            "CLIENT_WEB_OUTPUT": "/home/crespo/devp/osparc-simcore/services/static-webserver/client/source-output",
+            "DATCORE_ADAPTER_API_VERSION": "0.1.0-alpha",
+            "DIRECTOR_API_VERSION": "0.1.0",
+            "DIRECTOR_V2_API_VERSION": "2.0.0",
+            "DOCKER_IMAGE_TAG": "production",
+            "DOCKER_REGISTRY": "local",
+            "S3_ENDPOINT": "http://127.0.0.1:9001",
+            "STORAGE_API_VERSION": "0.2.1",
+            "SWARM_HOSTS": "",
+            "SWARM_STACK_NAME": "master-simcore",
+            "SWARM_STACK_NAME_NO_HYPHEN": "master_simcore",
+            "VCS_REF_CLIENT": "99b8022d2",
+            "VCS_STATUS_CLIENT": "'modified/untracked'",
+            "VCS_URL": "[email protected]:pcrespov/osparc-simcore.git",
+            "WEBSERVER_API_VERSION": "0.7.0",
+        },
+    )
+
+
+@pytest.fixture
+def mock_env_dockerfile_build(monkeypatch: pytest.MonkeyPatch) -> EnvVarsDict:
+    #
+    # docker run -it --hostname "{{.Node.Hostname}}-{{.Service.Name}}-{{.Task.Slot}}" local/webserver:production printenv
+    #
+    return setenvs_from_envfile(
+        monkeypatch,
+        """\
+        GPG_KEY=123456789123456789
+        HOME=/home/scu
+        HOSTNAME=osparc-master-55-master-simcore_master_webserver-1
+        IS_CONTAINER_CONTEXT=Yes
+        LANG=C.UTF-8
+        PATH=/home/scu/.venv/bin:/usr/local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
+        PWD=/home/scu
+        PYTHON_GET_PIP_SHA256=6123659241292b2147b58922b9ffe11dda66b39d52d8a6f3aa310bc1d60ea6f7
+        PYTHON_GET_PIP_URL=https://github.com/pypa/get-pip/raw/a1675ab6c2bd898ed82b1f58c486097f763c74a9/public/get-pip.py
+        PYTHON_PIP_VERSION=21.1.3
+        PYTHON_VERSION=3.11.9
+        PYTHONDONTWRITEBYTECODE=1
+        PYTHONOPTIMIZE=TRUE
+        SC_BOOT_MODE=production
+        SC_BUILD_DATE=2022-01-09T12:26:29Z
+        SC_BUILD_TARGET=production
+        SC_HEALTHCHECK_INTERVAL=30
+        SC_HEALTHCHECK_RETRY=3
+        SC_USER_ID=8004
+        SC_USER_NAME=scu
+        SC_VCS_REF=dd536f998
+        [email protected]:ITISFoundation/osparc-simcore.git
+        TERM=xterm
+        VIRTUAL_ENV=/home/scu/.venv
+    """,
+    )
+
+
+@pytest.fixture
+def mock_webserver_service_environment(
+    monkeypatch: pytest.MonkeyPatch,
+    mock_env_makefile: EnvVarsDict,  # pylint: disable=redefined-outer-name
+    mock_env_devel_environment: EnvVarsDict,  # pylint: disable=redefined-outer-name
+    mock_env_dockerfile_build: EnvVarsDict,  # pylint: disable=redefined-outer-name
+    mock_env_deployer_pipeline: EnvVarsDict,  # pylint: disable=redefined-outer-name
+) -> EnvVarsDict:
+    """
+    Mocks environment produce in the docker compose config with a .env (.env-devel)
+    and launched with a makefile
+    """
+    # @docker compose config (overrides)
+    # TODO: get from docker compose config
+    # r'- ([A-Z2_]+)=\$\{\1:-([\w-]+)\}'
+
+    # - .env-devel + docker-compose service environs
+    #     hostname: "{{.Node.Hostname}}-{{.Service.Name}}-{{.Task.Slot}}"
+
+    #     environment:
+    #         - CATALOG_HOST=${CATALOG_HOST:-catalog}
+    #         - CATALOG_PORT=${CATALOG_PORT:-8000}
+    #         - DIAGNOSTICS_MAX_AVG_LATENCY=10
+    #         - DIAGNOSTICS_MAX_TASK_DELAY=30
+    #         - DIRECTOR_PORT=${DIRECTOR_PORT:-8080}
+    #         - DIRECTOR_V2_HOST=${DIRECTOR_V2_HOST:-director-v2}
+    #         - DIRECTOR_V2_PORT=${DIRECTOR_V2_PORT:-8000}
+    #         - STORAGE_HOST=${STORAGE_HOST:-storage}
+    #         - STORAGE_PORT=${STORAGE_PORT:-8080}
+    #         - SWARM_STACK_NAME=${SWARM_STACK_NAME:-simcore}
+    #         - WEBSERVER_LOGLEVEL=${LOG_LEVEL:-WARNING}
+    #     env_file:
+    #         - ../.env
+    mock_envs_docker_compose_environment = setenvs_from_dict(
+        monkeypatch,
+        {
+            # Emulates MYVAR=${MYVAR:-default}
+            "CATALOG_HOST": os.environ.get("CATALOG_HOST", "catalog"),
+            "CATALOG_PORT": os.environ.get("CATALOG_PORT", "8000"),
+            "DIAGNOSTICS_MAX_AVG_LATENCY": "30",
+            "DIRECTOR_PORT": os.environ.get("DIRECTOR_PORT", "8080"),
+            "DIRECTOR_V2_HOST": os.environ.get("DIRECTOR_V2_HOST", "director-v2"),
+            "DIRECTOR_V2_PORT": os.environ.get("DIRECTOR_V2_PORT", "8000"),
+            "STORAGE_HOST": os.environ.get("STORAGE_HOST", "storage"),
+            "STORAGE_PORT": os.environ.get("STORAGE_PORT", "8080"),
+            "SWARM_STACK_NAME": os.environ.get("SWARM_STACK_NAME", "simcore"),
+            "WEBSERVER_LOGLEVEL": os.environ.get("LOG_LEVEL", "WARNING"),
+            "SESSION_COOKIE_MAX_AGE": str(7 * 24 * 60 * 60),
+        },
+    )
+
+    return (
+        mock_env_makefile
+        | mock_env_devel_environment
+        | mock_env_dockerfile_build
+        | mock_env_deployer_pipeline
+        | mock_envs_docker_compose_environment
+    )
+
+
 @pytest.fixture
 def mocked_login_required(mocker: MockerFixture):
 

diff --git a/services/web/server/tests/unit/isolated/test_application_settings.py b/services/web/server/tests/unit/isolated/test_application_settings.py
@@ -3,16 +3,11 @@
 # pylint:disable=no-name-in-module
 
 import json
-import os
 
 import pytest
 from aiohttp import web
 from models_library.utils.json_serialization import json_dumps
 from pydantic import HttpUrl, parse_obj_as
-from pytest_simcore.helpers.monkeypatch_envs import (
-    setenvs_from_dict,
-    setenvs_from_envfile,
-)
 from pytest_simcore.helpers.typing_env import EnvVarsDict
 from simcore_service_webserver.application_settings import (
     APP_SETTINGS_KEY,
@@ -21,144 +16,6 @@
 )
 
 
-@pytest.fixture
-def mock_env_devel_environment(
-    mock_env_devel_environment: EnvVarsDict, monkeypatch: pytest.MonkeyPatch
-) -> EnvVarsDict:
-    # Overrides to ensure dev-features are enabled testings
-    return mock_env_devel_environment | setenvs_from_dict(
-        monkeypatch,
-        envs={
-            "WEBSERVER_DEV_FEATURES_ENABLED": "1",
-        },
-    )
-
-
-@pytest.fixture
-def mock_env_makefile(monkeypatch: pytest.MonkeyPatch) -> EnvVarsDict:
-    """envvars produced @Makefile (export)"""
-    # TODO: add Makefile recipe 'make dump-envs' to produce the file we load here
-    return setenvs_from_dict(
-        monkeypatch,
-        {
-            "API_SERVER_API_VERSION": "0.3.0",
-            "BUILD_DATE": "2022-01-14T21:28:15Z",
-            "CATALOG_API_VERSION": "0.3.2",
-            "CLIENT_WEB_OUTPUT": "/home/crespo/devp/osparc-simcore/services/static-webserver/client/source-output",
-            "DATCORE_ADAPTER_API_VERSION": "0.1.0-alpha",
-            "DIRECTOR_API_VERSION": "0.1.0",
-            "DIRECTOR_V2_API_VERSION": "2.0.0",
-            "DOCKER_IMAGE_TAG": "production",
-            "DOCKER_REGISTRY": "local",
-            "S3_ENDPOINT": "http://127.0.0.1:9001",
-            "STORAGE_API_VERSION": "0.2.1",
-            "SWARM_HOSTS": "",
-            "SWARM_STACK_NAME": "master-simcore",
-            "SWARM_STACK_NAME_NO_HYPHEN": "master_simcore",
-            "VCS_REF_CLIENT": "99b8022d2",
-            "VCS_STATUS_CLIENT": "'modified/untracked'",
-            "VCS_URL": "[email protected]:pcrespov/osparc-simcore.git",
-            "WEBSERVER_API_VERSION": "0.7.0",
-        },
-    )
-
-
-@pytest.fixture
-def mock_env_dockerfile_build(monkeypatch: pytest.MonkeyPatch) -> EnvVarsDict:
-    #
-    # docker run -it --hostname "{{.Node.Hostname}}-{{.Service.Name}}-{{.Task.Slot}}" local/webserver:production printenv
-    #
-    return setenvs_from_envfile(
-        monkeypatch,
-        """\
-        GPG_KEY=123456789123456789
-        HOME=/home/scu
-        HOSTNAME=osparc-master-55-master-simcore_master_webserver-1
-        IS_CONTAINER_CONTEXT=Yes
-        LANG=C.UTF-8
-        PATH=/home/scu/.venv/bin:/usr/local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
-        PWD=/home/scu
-        PYTHON_GET_PIP_SHA256=6123659241292b2147b58922b9ffe11dda66b39d52d8a6f3aa310bc1d60ea6f7
-        PYTHON_GET_PIP_URL=https://github.com/pypa/get-pip/raw/a1675ab6c2bd898ed82b1f58c486097f763c74a9/public/get-pip.py
-        PYTHON_PIP_VERSION=21.1.3
-        PYTHON_VERSION=3.11.9
-        PYTHONDONTWRITEBYTECODE=1
-        PYTHONOPTIMIZE=TRUE
-        SC_BOOT_MODE=production
-        SC_BUILD_DATE=2022-01-09T12:26:29Z
-        SC_BUILD_TARGET=production
-        SC_HEALTHCHECK_INTERVAL=30
-        SC_HEALTHCHECK_RETRY=3
-        SC_USER_ID=8004
-        SC_USER_NAME=scu
-        SC_VCS_REF=dd536f998
-        [email protected]:ITISFoundation/osparc-simcore.git
-        TERM=xterm
-        VIRTUAL_ENV=/home/scu/.venv
-    """,
-    )
-
-
-@pytest.fixture
-def mock_webserver_service_environment(
-    monkeypatch: pytest.MonkeyPatch,
-    mock_env_makefile: EnvVarsDict,
-    mock_env_devel_environment: EnvVarsDict,
-    mock_env_dockerfile_build: EnvVarsDict,
-    mock_env_deployer_pipeline: EnvVarsDict,
-) -> EnvVarsDict:
-    """
-    Mocks environment produce in the docker compose config with a .env (.env-devel)
-    and launched with a makefile
-    """
-    # @docker compose config (overrides)
-    # TODO: get from docker compose config
-    # r'- ([A-Z2_]+)=\$\{\1:-([\w-]+)\}'
-
-    # - .env-devel + docker-compose service environs
-    #     hostname: "{{.Node.Hostname}}-{{.Service.Name}}-{{.Task.Slot}}"
-
-    #     environment:
-    #         - CATALOG_HOST=${CATALOG_HOST:-catalog}
-    #         - CATALOG_PORT=${CATALOG_PORT:-8000}
-    #         - DIAGNOSTICS_MAX_AVG_LATENCY=10
-    #         - DIAGNOSTICS_MAX_TASK_DELAY=30
-    #         - DIRECTOR_PORT=${DIRECTOR_PORT:-8080}
-    #         - DIRECTOR_V2_HOST=${DIRECTOR_V2_HOST:-director-v2}
-    #         - DIRECTOR_V2_PORT=${DIRECTOR_V2_PORT:-8000}
-    #         - STORAGE_HOST=${STORAGE_HOST:-storage}
-    #         - STORAGE_PORT=${STORAGE_PORT:-8080}
-    #         - SWARM_STACK_NAME=${SWARM_STACK_NAME:-simcore}
-    #         - WEBSERVER_LOGLEVEL=${LOG_LEVEL:-WARNING}
-    #     env_file:
-    #         - ../.env
-    mock_envs_docker_compose_environment = setenvs_from_dict(
-        monkeypatch,
-        {
-            # Emulates MYVAR=${MYVAR:-default}
-            "CATALOG_HOST": os.environ.get("CATALOG_HOST", "catalog"),
-            "CATALOG_PORT": os.environ.get("CATALOG_PORT", "8000"),
-            "DIAGNOSTICS_MAX_AVG_LATENCY": "30",
-            "DIRECTOR_PORT": os.environ.get("DIRECTOR_PORT", "8080"),
-            "DIRECTOR_V2_HOST": os.environ.get("DIRECTOR_V2_HOST", "director-v2"),
-            "DIRECTOR_V2_PORT": os.environ.get("DIRECTOR_V2_PORT", "8000"),
-            "STORAGE_HOST": os.environ.get("STORAGE_HOST", "storage"),
-            "STORAGE_PORT": os.environ.get("STORAGE_PORT", "8080"),
-            "SWARM_STACK_NAME": os.environ.get("SWARM_STACK_NAME", "simcore"),
-            "WEBSERVER_LOGLEVEL": os.environ.get("LOG_LEVEL", "WARNING"),
-            "SESSION_COOKIE_MAX_AGE": str(7 * 24 * 60 * 60),
-        },
-    )
-
-    return (
-        mock_env_makefile
-        | mock_env_devel_environment
-        | mock_env_dockerfile_build
-        | mock_env_deployer_pipeline
-        | mock_envs_docker_compose_environment
-    )
-
-
 @pytest.fixture
 def app_settings(
     mock_webserver_service_environment: EnvVarsDict,