Add instrumentation for fastapi-events

[melvinkcx]

Description

Hi 👋 , this PR adds instrumentation for fastapi-events. This is my first contribution to opentelemetry-python-contrib, any feedback is much appreciated. Thanks =)

Fixes # (issue)

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Test cases have been added to cover standard usage of dispatching and handling events with fastapi-events.

Does This PR Require a Core Repo Change?

• Yes. - Link to PR:
• No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added
• Documentation has been updated

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: melvinkcx / name: Melvin (8aacac0, d0f99a9, 9032073, 79286ef, c2ad91a, 7544011, 7ea369b)

[srikanthccv]

This looks like an instrumentation for a middleware package for FastAPI. I am not sure if this should be part of this repo. In an ideal scenario we want all the package authors to support OTEL natively but we maintain the instrumentation packages for various reasons. It already takes lot of compute resources to run the contrib tests and maintenance burden. I would suggest you pursue the author of the original middleware package to include the instrumentation support directly instead of writing another layer on top of it.

[ocelotl]

This looks like an instrumentation for a middleware package for FastAPI. I am not sure if this should be part of this repo. In an ideal scenario we want all the package authors to support OTEL natively but we maintain the instrumentation packages for various reasons. It already takes lot of compute resources to run the contrib tests and maintenance burden. I would suggest you pursue the author of the original middleware package to include the instrumentation support directly instead of writing another layer on top of it.

@srikanthccv Please discuss this topic in tomorrow SIG. I wont' be able to join but wanted to state this:

I'm personally not sure if we should reject this contribution on the grounds above because:

1. It is not clearly determined what counts as an instrumentation we should keep in the contrib repo and what not (I would prefer for instrumentations to be each in their own individual repo but that is another discussion topic), maybe this can be included in the current fastapi instrumentation?
2. It would be ideal if the OTel instrumentation is "natively" included in each third party library/framework, but unfortunately we have no control over that. If the third party library/framework community refuses to include it for whatever reason, users will be impacted by the lack of an alternative to do this kind of instrumentation.

Thanks! ✌️

[melvinkcx]

Hi @srikanthccv @ocelotl 👋, thanks for the feedback.

I am the original author of fastapi-events, it is totally possible for me to add OTel support to it.

However, I was under the impression that instrumentation libraries should be added to this repo and it (ideally) serves as a collection of all available instrumentation. Perhaps it is worth updating the contributing guide to mention this to future contributors?

[srikanthccv]

If you the author of the package then you should directly add it directly. The instrumentations are maintained here because package authors are/were not ready to support OTEL for various reasons. Some showed interest but didn't make much progress for example falconry/falcon#1828 . What serves as collection/index for instrumentations is this https://opentelemetry.io/registry/. If a package supports OTEL directly it should be listed there by adding entry.

[melvinkcx]

@srikanthccv Got it! It makes a lot of sense to me now.

[srikanthccv]

Yeah, people have this misconception that the instrumentaion should live here. That's not what we want to happen in the long run. Maybe my initial wording is not good I didn't mean to convey reject this but rather encourage you to pursue adding this support natively in the package (didn't know you are also the authour). There are number of reasons why I suggest this, the PRs for instrumentations do not get fast reviews because maintainers are busy and are not experts with specific library. And issues don't get addressed fast for the same reason. They pile up overtime and you can see this already in this repo. An regular python programmer can themselves write and publish opentelemetry-instrumentation-X on their own. There is restriction on that. Essentially I would like to see the community to pick up momentum and grow itself without having to depend on opentelemtry python team.

[srikanthccv]

@melvinkcx Appreciate the support for OTEL baked in directly https://github.com/melvinkcx/fastapi-events/blob/14f0b0c0109032ac3c5fa3a1df81ecb5719df18c/README.md#opentelemetry-otel-support. Hence closing this.