fastapi-slim support #2683

gryevns · 2024-07-09T13:59:39Z

What problem do you want to solve?

FastAPI now includes a new CLI dependency by default. A new package fastapi-slim was created which excludes the fastapi-cli dependency. We ideally want to use the new fastapi-slim package and the existing open telemetry fastapi instrumentation.

When trying to use this combination the open telemetry instrumentation fails to initialise because the expected package name is now different:

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py

Line 16 in eb8e456

_instruments = ("fastapi ~= 0.58",)

'DependencyConflict: requested: "fastapi ~= 0.58" but found: "None"'

Describe the solution you'd like

The open telemetry instrumentation for FastAPI should work with the fastapi-slim package and/or the fastapi package.

Describe alternatives you've considered

Can the FastAPI instrumentation be updated to work with either of the FastAPI packages or alternatively remove the check for the dependency completely?

Additional Context

No response

Would you like to implement a fix?

None

The text was updated successfully, but these errors were encountered:

xrmx · 2024-07-11T08:12:22Z

To anyone willing to tackle this, fixing it requires updating the package metadata in pyproject.toml

zhihali · 2024-07-11T17:48:00Z

Will open a PR for it

xrmx · 2024-07-11T20:09:43Z

~~Ah the problem is we currently are not able to express the OR dependency~~ The OR is fine, it's the AND we are missing.

zhihali · 2024-07-11T22:34:09Z

Hi @gryevns, could you tell me how did you get this error? I would like to reproduce it in my laptop but failed.

I run the small test with only installed fastapi-slim

from fastapi import FastAPI
from opentelemetry import trace
from opentelemetry.instrumentation.fastapi import FastAPIInstrumentor
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import ConsoleSpanExporter, BatchSpanProcessor

# Set up OpenTelemetry
trace.set_tracer_provider(TracerProvider())
tracer = trace.get_tracer(__name__)

# Set up logging of spans to console
span_processor = BatchSpanProcessor(ConsoleSpanExporter())
trace.get_tracer_provider().add_span_processor(span_processor)

app = FastAPI()

@app.get("/")
async def root():
    return {"message": "Hello World"}

@app.get("/items/{item_id}")
async def read_item(item_id: int):
    with tracer.start_as_current_span("read_item"):
        return {"item_id": item_id}

# Instrument FastAPI
FastAPIInstrumentor.instrument_app(app)

if __name__ == "__main__":
    import uvicorn
    uvicorn.run(app, host="0.0.0.0", port=8000)

didn't find the error 'DependencyConflict: requested: "fastapi ~= 0.58" but found: "None"'

zhihali · 2024-07-11T22:42:23Z

~~Ah the problem is we currently are not able to express the OR dependency~~ The OR is fine, it's the AND we are missing.

Hey @xrmx, could you help have a look if this change looks good?
my idea is to update the instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py

from

_instruments = ("fastapi ~= 0.58",)

to

_instruments = (
    "fastapi ~= 0.58.0",
    "fastapi-slim ~= 0.111.0",
)

and update the instrumentation/opentelemetry-instrumentation-fastapi/pyproject.toml
from

[project.optional-dependencies]
instruments = [
  "fastapi ~= 0.58",
]

to

[project.optional-dependencies]
instruments = [
    "fastapi ~= 0.58.0",
    "fastapi-slim ~= 0.111.0",
]

xrmx · 2024-07-12T07:13:04Z

@zhihali that looks correct, then you need to run tox -e generate to update the mapping used by opentelemetry-bootstrap.

gryevns · 2024-07-12T08:19:11Z

Hi @gryevns, could you tell me how did you get this error? I would like to reproduce it in my laptop but failed.

I run the small test with only installed fastapi-slim

from fastapi import FastAPI
from opentelemetry import trace
from opentelemetry.instrumentation.fastapi import FastAPIInstrumentor
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import ConsoleSpanExporter, BatchSpanProcessor

# Set up OpenTelemetry
trace.set_tracer_provider(TracerProvider())
tracer = trace.get_tracer(__name__)

# Set up logging of spans to console
span_processor = BatchSpanProcessor(ConsoleSpanExporter())
trace.get_tracer_provider().add_span_processor(span_processor)

app = FastAPI()

@app.get("/")
async def root():
    return {"message": "Hello World"}

@app.get("/items/{item_id}")
async def read_item(item_id: int):
    with tracer.start_as_current_span("read_item"):
        return {"item_id": item_id}

# Instrument FastAPI
FastAPIInstrumentor.instrument_app(app)

if __name__ == "__main__":
    import uvicorn
    uvicorn.run(app, host="0.0.0.0", port=8000)

didn't find the error 'DependencyConflict: requested: "fastapi ~= 0.58" but found: "None"'

@zhihali if you change FastAPIInstrumentor.instrument_app(app) to FastAPIInstrumentor().instrument() you should see the error logged when starting the app server.

$ python3 main.py
DependencyConflict: requested: "fastapi ~= 0.58" but found: "None"
INFO:     Started server process [97435]
INFO:     Waiting for application startup.
INFO:     Application startup complete.
INFO:     Uvicorn running on http://0.0.0.0:8001 (Press CTRL+C to quit)

$ pip list       
Package                               Version
------------------------------------- -------
annotated-types                       0.7.0
anyio                                 4.4.0
asgiref                               3.8.1
click                                 8.1.7
Deprecated                            1.2.14
fastapi-slim                          0.111.0
h11                                   0.14.0
idna                                  3.7
importlib_metadata                    7.1.0
opentelemetry-api                     1.25.0
opentelemetry-instrumentation         0.46b0
opentelemetry-instrumentation-asgi    0.46b0
opentelemetry-instrumentation-fastapi 0.46b0
opentelemetry-sdk                     1.25.0
opentelemetry-semantic-conventions    0.46b0
opentelemetry-util-http               0.46b0
pip                                   23.1.2
pydantic                              2.8.2
pydantic_core                         2.20.1
setuptools                            65.5.0
sniffio                               1.3.1
starlette                             0.37.2
typing_extensions                     4.12.2
uvicorn                               0.30.1
wrapt                                 1.16.0
zipp                                  3.19.2

gryevns · 2024-07-29T10:07:27Z

@zhihali

I don't believe #2702 fixes this issue, given the latest release and the reproduction code above you will still see DependencyConflict: requested: "fastapi ~= 0.58" but found: "None" output to the console.

The issue seems to occur because the following resolves as true:

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py

Lines 288 to 289 in c45a620

    
           if find_spec("fastapi") is not None: 
        
               return (_fastapi,)

However, the call when fetching the distrubution (fastapi) here will raise the DependencyConflict exception:

opentelemetry-python-contrib/opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py

Line 50 in c45a620

get_distribution(dep)

xrmx · 2024-07-29T10:16:46Z

@gryevns thanks for testing.

Is importlib.util.find_spec good enough for get_dependency_conflicts?

gryevns · 2024-07-29T10:26:45Z

@gryevns thanks for testing.

Is importlib.util.find_spec good enough for get_dependency_conflicts?

Not sure I understand your question. This approach works for me:

    def instrumentation_dependencies(self) -> Collection[str]:
        if find_spec("fastapi", package="fastapi-slim") is not None:
            return (_fastapi_slim,)
        if find_spec("fastapi") is not None:
            return (_fastapi,)
        return _instruments

xrmx · 2024-07-29T10:31:53Z

I thought you meant that the issue was in the get_dependency_conflicts function more than in the dependencies

gryevns · 2024-07-29T13:33:47Z

After further debugging it seems we'd need to look at the distributions to work out if fastapi-slim is installed. I am having trouble working out how the TestAutoInstrumentationLogic::test_instrumentation test works and why this passes for the new fastapi-slim tox config.

    def instrumentation_dependencies(self) -> Collection[str]:
        from pkg_resources import get_distribution
        try:
            get_distribution("fastapi-slim")
            return (_fastapi_slim,)
        except:
            ...

        if find_spec("fastapi") is not None:
            return (_fastapi,)

        # If neither is installed, return both as potential dependencies
        return _instruments

xrmx · 2024-07-30T13:12:35Z

I think it's because of how tox works:

diff --git a/tox.ini b/tox.ini
index 4ba434b2..353f383b 100644
--- a/tox.ini
+++ b/tox.ini
@@ -546,6 +546,7 @@ commands_pre =
   fastapi: pip install opentelemetry-sdk@{env:CORE_REPO}\#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk
   fastapi: pip install opentelemetry-test-utils@{env:CORE_REPO}\#egg=opentelemetry-test-utils&subdirectory=tests/opentelemetry-test-utils
   fastapi: pip install -r {toxinidir}/instrumentation/opentelemetry-instrumentation-fastapi/test-requirements.txt
+  fastapi-slim: pip uninstall fastapi
   fastapi-slim: pip install opentelemetry-api@{env:CORE_REPO}\#egg=opentelemetry-api&subdirectory=opentelemetry-api
   fastapi-slim: pip install opentelemetry-semantic-conventions@{env:CORE_REPO}\#egg=opentelemetry-semantic-conventions&subdirectory=opentelemetry-semantic-conventions
   fastapi-slim: pip install opentelemetry-sdk@{env:CORE_REPO}\#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk

tox -e py310-test-instrumentation-fastapi-slim:

...
py310-test-instrumentation-fastapi-slim: commands_pre[5]> pip uninstall fastapi
Found existing installation: fastapi 0.109.2
Uninstalling fastapi-0.109.2:
  Would remove:
    /home/rm/src/opentelemetry-python-contrib/.tox/py310-test-instrumentation-fastapi-slim/lib/python3.10/site-packages/fastapi-0.109.2.dist-info/*
    /home/rm/src/opentelemetry-python-contrib/.tox/py310-test-instrumentation-fastapi-slim/lib/python3.10/site-packages/fastapi/*

And 6 failed, 34 passed, 2 skipped, 1 warning in 1.11s

smoke · 2024-08-06T08:36:23Z

Implementation of this feature introduces bad bug that now when you have only fastapi installed auto-instrumentation fails with
Skipping instrumentation fastapi: DependencyConflict: requested: "fastapi-slim~=0.111.0" but found: "None" (use OTEL_PYTHON_LOG_LEVEL=debug as #1745 is not yet available)

So wrongly it now requires both fastapi AND fastapi-slim packages to be installed, instead of working like OR

Additionally it seems FastAPI are moving away from fastapi-slim to fastapi and fastapi[standard] instead - see https://fastapi.tiangolo.com/release-notes/#01120

Here repository with reproduction https://github.com/smoke/fastapi-autoinstrumentation-fails

xrmx · 2024-08-06T08:53:14Z

@smoke I don't think such tragic tone is needed given the issue is closed with a pr fixing the issue already.

Nice hint about the rename, if fastapi-slim is gone then we can remove some lines of code I guess :)

gryevns · 2024-08-06T08:55:36Z

Because FastAPI has reverted this packaging decision maybe we can revert back to only supporting fastapi which is simpler?

smoke · 2024-08-06T12:42:48Z

@smoke I don't think such tragic tone is needed given the issue is closed with a pr fixing the issue already.

Nice hint about the rename, if fastapi-slim is gone then we can remove some lines of code I guess :)

@xrmx The issue still exists with 0.47b0, here reproduction https://github.com/smoke/fastapi-autoinstrumentation-fails
and I am sure it exists with 0.48b0.dev for now as well.

The thing is that _load_instrumentors uses get_dist_dependency_conflicts(entry_point.dist) that only checks the dependencies as defined in the package metadata and it uses AND logic, requiring all of them to be met,
I see no option to support OR mode as intended without modifying it.

So probably the next best and easy thing is to remove the fastapi-slim support, especially given the prev comment.

emdneto · 2024-08-06T14:43:21Z

@smoke This seems to be the best approach for me. Would you be willing to submit a PR for this?

smoke · 2024-08-07T07:14:30Z

@smoke This seems to be the best approach for me. Would you be willing to submit a PR for this?

@emdneto @xrmx @gryevns Please find the PR removing fastapi-slim support here #2783

gryevns added the feature-request label Jul 9, 2024

xrmx added the good first issue Good for newcomers label Jul 11, 2024

This was referenced Jul 12, 2024

Teach fastapi instrumentation about fastapi-slim #2702

Merged

REQUEST: New membership for zhihali open-telemetry/community#2214

Closed

lzchen closed this as completed in #2702 Jul 23, 2024

xrmx mentioned this issue Jul 30, 2024

instrumentation/fastapi: fix fastapi-slim support #2756

Merged

11 tasks

smoke mentioned this issue Aug 7, 2024

fix: fastapi auto-instrumentation by removing fastapi-slim support #2783

Merged

10 tasks

jeremydvoss mentioned this issue Sep 4, 2024

Consider altering "instruments" set to avoid breaking changes #2853

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fastapi-slim support #2683

fastapi-slim support #2683

gryevns commented Jul 9, 2024 •

edited

Loading

xrmx commented Jul 11, 2024

zhihali commented Jul 11, 2024

xrmx commented Jul 11, 2024 •

edited

Loading

zhihali commented Jul 11, 2024

zhihali commented Jul 11, 2024

xrmx commented Jul 12, 2024

gryevns commented Jul 12, 2024

gryevns commented Jul 29, 2024 •

edited

Loading

xrmx commented Jul 29, 2024 •

edited

Loading

gryevns commented Jul 29, 2024

xrmx commented Jul 29, 2024

gryevns commented Jul 29, 2024

xrmx commented Jul 30, 2024

smoke commented Aug 6, 2024 •

edited

Loading

xrmx commented Aug 6, 2024

gryevns commented Aug 6, 2024

smoke commented Aug 6, 2024 •

edited

Loading

emdneto commented Aug 6, 2024

smoke commented Aug 7, 2024 •

edited

Loading

fastapi-slim support #2683

fastapi-slim support #2683

Comments

gryevns commented Jul 9, 2024 • edited Loading

What problem do you want to solve?

Describe the solution you'd like

Describe alternatives you've considered

Additional Context

Would you like to implement a fix?

xrmx commented Jul 11, 2024

zhihali commented Jul 11, 2024

xrmx commented Jul 11, 2024 • edited Loading

zhihali commented Jul 11, 2024

zhihali commented Jul 11, 2024

xrmx commented Jul 12, 2024

gryevns commented Jul 12, 2024

gryevns commented Jul 29, 2024 • edited Loading

xrmx commented Jul 29, 2024 • edited Loading

gryevns commented Jul 29, 2024

xrmx commented Jul 29, 2024

gryevns commented Jul 29, 2024

xrmx commented Jul 30, 2024

smoke commented Aug 6, 2024 • edited Loading

xrmx commented Aug 6, 2024

gryevns commented Aug 6, 2024

smoke commented Aug 6, 2024 • edited Loading

emdneto commented Aug 6, 2024

smoke commented Aug 7, 2024 • edited Loading

gryevns commented Jul 9, 2024 •

edited

Loading

xrmx commented Jul 11, 2024 •

edited

Loading

gryevns commented Jul 29, 2024 •

edited

Loading

xrmx commented Jul 29, 2024 •

edited

Loading

smoke commented Aug 6, 2024 •

edited

Loading

smoke commented Aug 6, 2024 •

edited

Loading

smoke commented Aug 7, 2024 •

edited

Loading