docs(fastapi): add usage example and setup order guidance#4314
docs(fastapi): add usage example and setup order guidance#4314samaarr wants to merge 4 commits intoopen-telemetry:mainfrom
Conversation
The existing README had no usage example. New contributors and users had no indication that TracerProvider must be configured before instrument_app() is called, or that instrumentation should happen before routes are defined. Add a minimal working example showing correct setup order, a note clarifying the instrumentation timing requirement, and an example for excluding URLs from instrumentation.
|
|
||
| .. note:: | ||
|
|
||
| ``instrument_app()`` must be called after the ``FastAPI`` instance is created |
There was a problem hiding this comment.
Unsure of actual functionality, but this text section seems to contradict the previous section. Correct me if I am wrong, but you initially say instrument_app should be called before defining routes, but here you say calling instrument_app after defining routes is supported.
There was a problem hiding this comment.
instrument_app() must be called after the FastAPI instance is
created but before any routes are defined, so that the instrumentation
middleware is in place before the app starts serving requests.
There was a problem hiding this comment.
Yes, there is still a contradiction between this section and lines 22-24.
There was a problem hiding this comment.
Thank you for your input, I've removed the Usage section from the readme entirely and moved the example into the module docstring instead, per emdneto's suggestion. The ordering note in the docstring now reads: "Instrument after creating the app and before serving requests; calling it after routes are registered is also supported." I think there is no contradiction now.
|
|
||
| .. code-block:: python | ||
|
|
||
| FastAPIInstrumentor.instrument_app(app, excluded_urls="/healthz,/metrics") |
There was a problem hiding this comment.
Let's go ahead and provide examples for urls that would be excluded according to these params to verify the exclusion of partial matches, if that is the case. The README for falcon instrumentation is a good example: https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-falcon/README.rst.
There was a problem hiding this comment.
Updated. The docstring now shows both a partial match example (with a comment explaining that "healthz" matches /healthz, /internal/healthz, etc.) and an anchored pattern example for exact path matching.
emdneto
left a comment
emdneto
left a comment
There was a problem hiding this comment.
I think it's better to improve whatever we have here
https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/fastapi/fastapi.html
…nd excluded_urls partial match guidance
|
Thanks @emdneto and @xrmx. I moved the example out of the README and into the module docstring so it shows up on the readthedocs page. README reverted. @JWinermaSplunk the ordering contradiction and excluded_urls partial match examples are also addressed in the updated docstring. |
|
@samaarr I don't see the README.rst changes reverted |
|
Done. commit 99dec59 restores the README to the original. The diff now only touches init.py. |
|
This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment. |
|
This PR has been closed due to inactivity. Please reopen if you would like to continue working on it. |
github-project-automation
Bot
moved this from Reviewed PRs that need fixes
to Done
in Python PR digest
4 participants
The existing README contained no usage example, leaving users to
figure out correct setup from the source code or external blog posts.
This PR adds:
I ran into the setup ordering issue while building a demo project with
FastAPI + OTel Collector and found the lack of any example in the README
was a common stumbling block.
Fixes: none (docs improvement)