trallnag / prometheus-fastapi-instrumentator

Instrument your FastAPI with Prometheus metrics.
ISC License
948 stars 84 forks source link

feat: Make instrumentator work with just Starlette #288

Closed mvanderlee closed 7 months ago

mvanderlee commented 7 months ago

What does this do?

This PR removes FastAPI and uses Starlette instead (which FastAPI extends).

Why do we need it?

This means that non-FastAPI projects don't have to install all it's dependencies.

Who is this for?

Non-FastAPI users

Linked issues

Fixes #280

Reviewer notes

--

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.79%. Comparing base (885c80c) to head (a1f989b).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #288 +/- ## ========================================== + Coverage 95.68% 95.79% +0.10% ========================================== Files 5 5 Lines 348 357 +9 ========================================== + Hits 333 342 +9 Misses 15 15 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

trallnag commented 7 months ago

Hi, thanks for proposing and implementing this

Note that it is not possible to remove any trace of FastAPI from the instrumentator, as the expose() method relies on FastAPI-specific parameters. I could remove it, but I would prefer to keep it backwards compatible.

Nevertheless I managed to remove the dependency on the fastapi package by dynamically checking if the app is Starlette or FastAPI instance and only importing fastapi, if it is a FastAPI app. I added the changes to your PR.

It should be backwards compatible

mvanderlee commented 7 months ago

Nice, awesome work @trallnag!