fix: update opentelemetry dependencies to patch security vulnerability (

#531) * fix: update opentelemetry dependencies * refactor: sensitive/flaky tests --------- Co-authored-by: saartochner <[email protected]>
lumigo-io · Oct 23, 2023 · 0e10e7e · 0e10e7e
1 parent 73844e0
commit 0e10e7e
Show file tree

Hide file tree

Showing 41 changed files with 646 additions and 536 deletions.
diff --git a/.github/workflows/push-actions.yml b/.github/workflows/push-actions.yml
@@ -26,7 +26,7 @@ jobs:
           # The task outputs the list of tasks on stdout, each entry separated by a `\n` character.
           # The task output is piped into the `jq` command, that formats it as a JSON list, as the
           # we need a JSON datastructure later on for dynamic matrixes.
-          echo "integration_tests=$(nox -e list_integration_tests_ci | jq -Rnc '[inputs]')" >> $GITHUB_OUTPUT
+          echo "integration_tests=$(python -m nox -e list_integration_tests_ci | jq -Rnc '[inputs]')" >> $GITHUB_OUTPUT
 
   version-testing:
     needs: [list-instrumentations]

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -25,7 +25,8 @@ It might be necessary to throw in a `brew tap homebrew/core` along the way.
 
 ### PyCharm Users
 
-If you are using pycharm, ensure that you set it to use the virtualenv virtual environment manager. This is available in the menu under `PyCharm -> Preferences -> Project -> Interpreter`
+If you are using PyCharm, ensure that you set it to use the virtualenv virtual environment manager.
+This is available in the menu under `PyCharm -> Settings -> Project -> Interpreter`.
 
 ## Running the test suite
 

diff --git a/noxfile.py b/noxfile.py
@@ -10,10 +10,11 @@
 from xml.etree import ElementTree
 
 import nox
-import psutil
 import requests
 import yaml
 
+from src.test.test_utils.processes import kill_process, wait_for_app_start
+
 # Ensure nox can load local packages
 repo_dir = os.path.dirname(__file__)
 if repo_dir not in sys.path:
@@ -41,10 +42,10 @@ def create_component_tempfile(name: str):
     return temp_file.name
 
 
-def create_it_tempfile(name: str):
+def create_it_tempfile(name: str, prefix: str = "temp_"):
     temp_file = tempfile.NamedTemporaryFile(
         suffix=".txt",
-        prefix="temp_",
+        prefix=prefix,
         dir=os.path.abspath(f"src/test/integration/{name}/"),
         delete=False,
     )
@@ -160,11 +161,6 @@ def dependency_versions_to_be_tested(
     ]
 
 
-def wait_for_app_start():
-    # TODO Make this deterministic
-    time.sleep(8)
-
-
 @nox.session()
 def list_integration_tests_ci(session):
     integration_tests = {
@@ -363,19 +359,6 @@ def integration_tests_fastapi(
         session.install("-r", OTHER_REQUIREMENTS)
 
         try:
-            session.run(
-                "sh",
-                "./scripts/start_uvicorn",
-                env={
-                    "AUTOWRAPT_BOOTSTRAP": "lumigo_opentelemetry",
-                    "LUMIGO_DEBUG_SPANDUMP": temp_file,
-                    "OTEL_SERVICE_NAME": "app",
-                },
-                external=True,
-            )  # One happy day we will have https://github.com/wntrblm/nox/issues/198
-
-            wait_for_app_start()
-
             session.run(
                 "pytest",
                 "--tb",
@@ -389,69 +372,18 @@ def integration_tests_fastapi(
                 },
             )
         finally:
-            kill_process_and_clean_outputs(temp_file, "uvicorn", session)
+            clean_outputs(temp_file, session)
 
 
 @nox.session(python=python_versions())
 def component_tests(session):
-    component_tests_attr_max_size(
-        session=session,
-        fastapi_version="0.78.0",  # arbitrary version
-        uvicorn_version="0.16.0",  # TODO don't update, see https://lumigo.atlassian.net/browse/RD-11466
-    )
     component_tests_execution_tags(
         session=session,
         fastapi_version="0.78.0",  # arbitrary version
         uvicorn_version="0.16.0",  # TODO don't update, see https://lumigo.atlassian.net/browse/RD-11466
     )
 
 
-def component_tests_attr_max_size(
-    session,
-    fastapi_version,
-    uvicorn_version,
-):
-    install_package("uvicorn", uvicorn_version, session)
-    install_package("fastapi", fastapi_version, session)
-
-    session.install(".")
-
-    temp_file = create_component_tempfile("attr_max_size")
-    with session.chdir("src/test/components"):
-        session.install("-r", OTHER_REQUIREMENTS)
-
-        try:
-            session.run(
-                "sh",
-                "./scripts/start_uvicorn",
-                env={
-                    "AUTOWRAPT_BOOTSTRAP": "lumigo_opentelemetry",
-                    "LUMIGO_DEBUG_SPANDUMP": temp_file,
-                    "OTEL_SERVICE_NAME": "app",
-                    "OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT": "1",
-                },
-                external=True,
-            )  # One happy day we will have https://github.com/wntrblm/nox/issues/198
-
-            wait_for_app_start()
-
-            session.run(
-                "pytest",
-                "--tb",
-                "native",
-                "--log-cli-level=INFO",
-                "--color=yes",
-                "-v",
-                "./tests/test_attr_max_size.py",
-                env={
-                    "LUMIGO_DEBUG_SPANDUMP": temp_file,
-                    "OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT": "1",
-                },
-            )
-        finally:
-            kill_process_and_clean_outputs(temp_file, "uvicorn", session)
-
-
 def component_tests_execution_tags(
     session,
     fastapi_version,
@@ -467,19 +399,6 @@ def component_tests_execution_tags(
         session.install("-r", OTHER_REQUIREMENTS)
 
         try:
-            session.run(
-                "sh",
-                "./scripts/start_uvicorn",
-                env={
-                    "AUTOWRAPT_BOOTSTRAP": "lumigo_opentelemetry",
-                    "LUMIGO_DEBUG_SPANDUMP": temp_file,
-                    "OTEL_SERVICE_NAME": "app",
-                },
-                external=True,
-            )  # One happy day we will have https://github.com/wntrblm/nox/issues/198
-
-            wait_for_app_start()
-
             session.run(
                 "pytest",
                 "--tb",
@@ -493,7 +412,7 @@ def component_tests_execution_tags(
                 },
             )
         finally:
-            kill_process_and_clean_outputs(temp_file, "uvicorn", session)
+            clean_outputs(temp_file, session)
 
 
 @nox.session()
@@ -638,29 +557,15 @@ def integration_tests_grpcio(
             "python", "-m", "pip", "install", "--upgrade", "pip", "setuptools", "wheel"
         )
 
-        server_spans = tempfile.NamedTemporaryFile(
-            suffix=".txt", prefix=create_it_tempfile("grpcio")
-        ).name
-        client_spans = tempfile.NamedTemporaryFile(
-            suffix=".txt", prefix=create_it_tempfile("grpcio")
-        ).name
+        base_span_file = create_it_tempfile("grpcio")
+        clean_outputs(base_span_file, session)
+
+        server_spans = create_it_tempfile("grpcio", prefix=base_span_file)
+        client_spans = create_it_tempfile("grpcio", prefix=base_span_file)
         with session.chdir("src/test/integration/grpcio"):
             session.install("-r", OTHER_REQUIREMENTS)
 
             try:
-                session.run(
-                    "sh",
-                    "./scripts/start_server",
-                    env={
-                        "AUTOWRAPT_BOOTSTRAP": "lumigo_opentelemetry",
-                        "LUMIGO_DEBUG_SPANDUMP": server_spans,
-                        "OTEL_SERVICE_NAME": "app",
-                    },
-                    external=True,
-                )  # One happy day we will have https://github.com/wntrblm/nox/issues/198
-
-                wait_for_app_start()
-
                 session.run(
                     "pytest",
                     "--tb",
@@ -677,7 +582,6 @@ def integration_tests_grpcio(
                     },
                 )
             finally:
-                kill_process("greeter_server.py")
                 clean_outputs(server_spans, session)
                 clean_outputs(client_spans, session)
 
@@ -1084,50 +988,6 @@ def kill_process_and_clean_outputs(full_path: str, process_name: str, session) -
     clean_outputs(full_path, session)
 
 
-def kill_process(process_name: str) -> None:
-    proc_name = "undefined"
-    cmd_line = "undefined"
-    try:
-        # Kill all processes with the given name
-        for proc in psutil.process_iter(
-            attrs=["pid", "name", "cmdline"], ad_value=None
-        ):
-            proc_name = proc.name()
-            if proc.status() == psutil.STATUS_ZOMBIE:
-                continue
-            # The python process is named "Python" on OS X and "uvicorn" on CircleCI
-            if proc_name == process_name:
-                print(f"Killing process with name {proc_name}...")
-                proc.kill()
-            elif proc_name.lower().startswith("python"):
-                # drop the first argument, which is the python executable
-                python_command_parts = proc.cmdline()[1:]
-                # the initial command part is the last part of the path
-                python_command_parts[0] = python_command_parts[0].split("/")[-1]
-                # combine the remaining arguments
-                command = " ".join(python_command_parts)
-                print(
-                    f"Evaluating process with name '{proc_name}' and command '{command}'..."
-                )
-                if (
-                    len(cmd_line) > 1
-                    and "nox" not in command
-                    and process_name in command
-                ):
-                    print(
-                        f"Killing process with name '{proc_name}' and command '{command}'..."
-                    )
-                    proc.kill()
-    except psutil.ZombieProcess as zp:
-        print(
-            f"Failed to kill zombie process '{proc_name}' (looking for {process_name}) with command line '{cmd_line}': {str(zp)}"
-        )
-    except psutil.NoSuchProcess as nsp:
-        print(
-            f"Failed to kill process '{proc_name}' (looking for {process_name}) with command line '{cmd_line}': {str(nsp)}"
-        )
-
-
 def clean_outputs(full_path: str, session) -> None:
     session.run("rm", "-f", full_path, external=True)
 

diff --git a/setup.py b/setup.py
@@ -18,25 +18,25 @@
         "protobuf>=3.13.0, <4.0.0",
         "wrapt>=1.11.0",
         "lumigo_core==0.0.6",
-        "opentelemetry-api==1.15.0",
-        "opentelemetry-sdk==1.15.0",
+        "opentelemetry-api==1.20.0",
+        "opentelemetry-sdk==1.20.0",
         "opentelemetry-sdk-extension-aws==2.0.1",
-        "opentelemetry-exporter-otlp-proto-http==1.15.0",
-        "opentelemetry-semantic-conventions==0.36b0",
-        "opentelemetry-instrumentation==0.36b0",
-        "opentelemetry-instrumentation-asgi==0.36b0",
-        "opentelemetry-instrumentation-boto==0.36b0",
-        "opentelemetry-instrumentation-fastapi==0.36b0",
-        "opentelemetry-instrumentation-flask==0.36b0",
-        "opentelemetry-instrumentation-grpc==0.36b0",
-        "opentelemetry-instrumentation-kafka-python==0.36b0",
-        "opentelemetry-instrumentation-pika==0.36b0",
-        "opentelemetry-instrumentation-psycopg2==0.36b0",
-        "opentelemetry-instrumentation-pymongo==0.36b0",
-        "opentelemetry-instrumentation-pymysql==0.36b0",
-        "opentelemetry-instrumentation-requests==0.36b0",
-        "opentelemetry-instrumentation-redis==0.36b0",
-        "opentelemetry-instrumentation-django==0.36b0",
+        "opentelemetry-exporter-otlp-proto-http==1.20.0",
+        "opentelemetry-semantic-conventions==0.41b0",
+        "opentelemetry-instrumentation==0.41b0",
+        "opentelemetry-instrumentation-asgi==0.41b0",
+        "opentelemetry-instrumentation-boto==0.41b0",
+        "opentelemetry-instrumentation-fastapi==0.41b0",
+        "opentelemetry-instrumentation-flask==0.41b0",
+        "opentelemetry-instrumentation-grpc==0.41b0",
+        "opentelemetry-instrumentation-kafka-python==0.41b0",
+        "opentelemetry-instrumentation-pika==0.41b0",
+        "opentelemetry-instrumentation-psycopg2==0.41b0",
+        "opentelemetry-instrumentation-pymongo==0.41b0",
+        "opentelemetry-instrumentation-pymysql==0.41b0",
+        "opentelemetry-instrumentation-requests==0.41b0",
+        "opentelemetry-instrumentation-redis==0.41b0",
+        "opentelemetry-instrumentation-django==0.41b0",
         # v4.7.1 is the last version that supports python 3.7
         "typing_extensions==4.7.1; python_version<'3.8'",
     ],

diff --git a/src/lumigo_opentelemetry/instrumentations/fastapi/__init__.py b/src/lumigo_opentelemetry/instrumentations/fastapi/__init__.py
@@ -13,6 +13,13 @@ def install_instrumentation(self) -> None:
         import wrapt
 
         from lumigo_opentelemetry import logger
+        from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware
+
+        wrapt.wrap_function_wrapper(
+            OpenTelemetryMiddleware,
+            "_get_otel_receive",
+            FastAPIParser.wrapt__get_otel_receive,
+        )
 
         @wrapt.patch_function_wrapper("fastapi", "FastAPI.__init__")
         def init_otel_middleware(wrapped, instance, args, kwargs):  # type: ignore
@@ -23,7 +30,6 @@ def init_otel_middleware(wrapped, instance, args, kwargs):  # type: ignore
                 FastAPIInstrumentor().instrument_app(
                     instance,
                     server_request_hook=FastAPIParser.server_request_hook,
-                    client_request_hook=FastAPIParser.client_request_hook,
                     client_response_hook=FastAPIParser.client_response_hook,
                 )
 

diff --git a/src/lumigo_opentelemetry/instrumentations/fastapi/parsers/__init__.py b/src/lumigo_opentelemetry/instrumentations/fastapi/parsers/__init__.py
@@ -1,6 +1,5 @@
 from typing import Dict, Any
 
-from lumigo_opentelemetry import logger
 from lumigo_opentelemetry.libs.general_utils import lumigo_safe_execute
 from lumigo_opentelemetry.libs.json_utils import (
     dump,
@@ -36,9 +35,20 @@ def server_request_hook(span: Span, scope: Dict[str, Any]) -> None:
             span.set_attributes(attributes)
 
     @staticmethod
-    def client_request_hook(span: Span, scope: Dict[Any, Any]) -> None:
-        with lumigo_safe_execute("FastAPIParser: client_request_hook"):
-            logger.debug(f"client_request_hook span: {span}, scope: {scope}")
+    def wrapt__get_otel_receive(original_func, instance, args, kwargs):  # type: ignore
+        original_otel_receive = original_func(*args, **kwargs)
+
+        async def new_otel_receive():  # type: ignore
+            return_value = await original_otel_receive()
+            with lumigo_safe_execute("FastAPIParser: new_otel_receive"):
+                with instance.tracer.start_as_current_span("receive_body") as send_span:
+                    send_span.set_attribute(
+                        "http.request.body",
+                        dump_with_context("requestBody", return_value),
+                    )
+            return return_value
+
+        return new_otel_receive
 
     @staticmethod
     def client_response_hook(span: Span, message: Dict[str, Any]) -> None:

diff --git a/src/lumigo_opentelemetry/instrumentations/redis/__init__.py b/src/lumigo_opentelemetry/instrumentations/redis/__init__.py
@@ -23,8 +23,9 @@ def request_hook(
             span: Span, instance: Connection, args: List[Any], kwargs: Dict[Any, Any]
         ) -> None:
             # a db.statement attribute is automatically added by the RedisInstrumentor
-            # when this hook is called, so we don't need to add anything manually
-            pass
+            # when this hook is called, but only includes the command name for some
+            # versions so we need to set it ourselves.
+            add_body_attribute(span, " ".join(args), "db.statement")
 
         def response_hook(span: Span, instance: Connection, response: Any) -> None:
             add_body_attribute(span, response, "db.response.body")