Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use opentelemetry-instrumentation-aiohttp-server from upstream #5838

Merged

Conversation

lubosmj
Copy link
Member

@lubosmj lubosmj commented Sep 25, 2024

closes #5833

@lubosmj lubosmj marked this pull request as ready for review September 27, 2024 06:12
@lubosmj
Copy link
Member Author

lubosmj commented Sep 27, 2024

I wonder if it is needed to apply this change because of autoinstrumentation:

--- a/pulpcore/app/wsgi.py
+++ b/pulpcore/app/wsgi.py
@@ -8,6 +8,7 @@ https://docs.djangoproject.com/en/3.2/howto/deployment/wsgi/
 """
 
 from django.core.wsgi import get_wsgi_application
+from opentelemetry.instrumentation.wsgi import OpenTelemetryMiddleware
 
 from pulpcore.app.entrypoint import using_pulp_api_worker
 
@@ -15,6 +16,7 @@ if not using_pulp_api_worker.get(False):
     raise RuntimeError("This app must be executed using pulpcore-api entrypoint.")
 
 application = get_wsgi_application()
+application = OpenTelemetryMiddleware(application)
 
 # Disabling Storage metrics until we find a solution to resource usage.
 # https://github.com/pulp/pulpcore/issues/5468
diff --git a/pulpcore/content/__init__.py b/pulpcore/content/__init__.py
index 14c65ea81..8b24272d7 100644
--- a/pulpcore/content/__init__.py
+++ b/pulpcore/content/__init__.py
@@ -20,6 +20,7 @@ from django.db.utils import (  # noqa: E402: module level not at top of file
     DatabaseError,
 )
 
+from opentelemetry.instrumentation.aiohttp_server import middleware as instrumentation
 from pulpcore.app.apps import pulp_plugin_configs  # noqa: E402: module level not at top of file
 from pulpcore.app.models import ContentAppStatus  # noqa: E402: module level not at top of file
 
@@ -29,7 +30,7 @@ from .authentication import authenticate  # noqa: E402: module level not at top
 
 log = logging.getLogger(__name__)
 
-app = web.Application(middlewares=[authenticate])
+app = web.Application(middlewares=[authenticate, instrumentation])
 
 CONTENT_MODULE_NAME = "content"

@lubosmj lubosmj force-pushed the use-upstream-aiohttp-server-otel-instrumentation branch from ee8fafa to 25b47fc Compare September 30, 2024 10:32
@lubosmj
Copy link
Member Author

lubosmj commented Sep 30, 2024

Seeing the docs for the wsgi instrumentation, I think we should follow the recommended usage: https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/wsgi/wsgi.html.

@lubosmj lubosmj force-pushed the use-upstream-aiohttp-server-otel-instrumentation branch from 25b47fc to dc25f69 Compare September 30, 2024 10:35
@lubosmj lubosmj requested a review from decko September 30, 2024 10:35
@lubosmj lubosmj marked this pull request as draft September 30, 2024 12:50
@lubosmj
Copy link
Member Author

lubosmj commented Sep 30, 2024

Putting on hold until we figure more things out.

@lubosmj lubosmj marked this pull request as ready for review October 2, 2024 14:00
@dkliban dkliban merged commit dcad800 into pulp:main Oct 2, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use an opentelemetry-python package to handle the aiohttp server instrumentation
2 participants