open-telemetry / opentelemetry-python-contrib

OpenTelemetry instrumentation for Python modules
https://opentelemetry.io
Apache License 2.0
648 stars 535 forks source link

FastAPI `uninstrument` disallows adding more middlewares with `fastapi@>=0.91` and `starlette@>=0.24` #2042

Open maxkomarychev opened 8 months ago

maxkomarychev commented 8 months ago

Uninstrument is breaking subsequent calls to add_middleware due to recent changes in starlette https://github.com/encode/starlette/pull/2017 and latest version of fastapi@>=0.91.

Call to build_middleware_stack makes an instance of FastAPI and starelette to think that application is already running

Describe your environment

Python 3.9.9

opentelemetry-distro = "^0.41b0"
opentelemetry-exporter-otlp = "^1.20.0"
opentelemetry-instrumentation-aws-lambda = "^0.41b0"
opentelemetry-instrumentation-dbapi = "^0.41b0"
opentelemetry-instrumentation-logging = "^0.41b0"
opentelemetry-instrumentation-sqlite3 = "^0.41b0"
opentelemetry-instrumentation-urllib = "^0.41b0"
opentelemetry-instrumentation-wsgi = "^0.41b0"
opentelemetry-instrumentation-asgi = "^0.41b0"
opentelemetry-instrumentation-fastapi = "^0.41b0"
opentelemetry-instrumentation-grpc = "^0.41b0"
opentelemetry-instrumentation-httpx = "^0.41b0"
opentelemetry-instrumentation-requests = "^0.41b0"
opentelemetry-instrumentation-sqlalchemy = "^0.41b0"
opentelemetry-instrumentation-urllib3 = "^0.41b0"

Steps to reproduce Describe exactly how to reproduce the error. Include a code sample if applicable.

from starlette.middleware.cors import CORSMiddleware
from opentelemetry.instrumentation.fastapi import FastAPIInstrumentor

app = FastAPI(...)

FastAPIInstrumentor.uninstrument_app(app)

app.add_middleware(
    CORSMiddleware,
    allow_credentials=True,
    allow_methods=["*"],
    allow_headers=["*"],
)

What is the expected behavior? What did you expect to see?

FastAPIInstrumentor.uninstrument_app should not break ability to add more middlewares

What is the actual behavior? What did you see instead?

Application crashes:

  File "/xxx/.venv/lib/python3.9/site-packages/starlette/applications.py", line 139, in add_middleware
    raise RuntimeError("Cannot add middleware after an application has started")

Additional context Add any other context about the problem here.

jordiandreu commented 3 months ago

The same happens with method FastAPIInstrumentor.instrument_app(app):

venv/lib64/python3.9/site-packages/opentelemetry/instrumentation/fastapi/init.py:232: in instrument_app

def add_middleware(self, middleware_class: typing.Type[_MiddlewareClass[P]], *args: P.args, **kwargs: P.kwargs, ) -> None:

    if self.middleware_stack is not None:  # pragma: no cover
        raise RuntimeError("Cannot add middleware after an application has started")
pamelafox commented 2 months ago

I also had this issue when I instrumented in lifespan startup event, here's my fix: https://github.com/pamelafox/staticmaps-function/pull/72