feat: move OTEL to a new 'InstrumentedFosite'

[vivshankar]

The recent change in [#761 ] introduces OTEL as part of Fosite. While this is valuable, this should be provided as a wrapper and not as a hard wired part of Fosite, which is meant to be a SDK. Also, popular instrumentation options and APM vendors have some level of support for OTEL but shine when their wrapper SDKs are used that do not always comply with OTEL interfaces. See New Relic and Instana SDKs for reference.

This change proposes that OTEL should be an opt-in as part of a wrapper implementation of Fosite that is called InstrumentedFosite under the otel package. It also offers a pattern that can be applied to any code blocks and handlers that should be instrumented to derive maximum benefits from a telemetry perspective. For example - there may be no value in instrumenting a code block that interfaces with no external components beyond the overall telemetry offered at a HTTP middleware level, whereas there may be a great deal of value instrumenting call outs to external APIs and databases.

Related Issue or Design Document

761

Checklist

• [x] I have read the contributing guidelines and signed the CLA.
• [x] I have referenced an issue containing the design document if my change introduces a new feature.
• [x] I have read the security policy.
• [x] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security vulnerability, I confirm that I got approval (please contact security@ory.sh) from the maintainers to push the changes.
• [x] I have added tests that prove my fix is effective or that my feature works.
• [x] I have added the necessary documentation within the code base (if appropriate).

Further comments

[james-d-elliott]

Tests seem to just be failing due ory/hydra using a PR in go.mod. I like this change, I did some experimentation with it in making github.com/ory/fosite/otel it's own module so it's entirely standalone however I could not get it working, maybe it's unnecessary due to pruning and github.com/ory/x contains the imports anyway.

[vivshankar]

The way we abstracted this for future proofing is to introduce a metrics interface that allows you to swap in providers as needed. OTEL is great but is not completely adopted by major vendors (referenced in my post and from actual usage of said vendor SDK and providers). Also the current implementation takes away any flexibility for adopters to use different libraries.

I am happy to take another stab at this to put it in Fosite directly if that will be acceptable.

With regards to adopters wrapping Fosite, the current implementation doesn't let me do that because OTEL is used directly without an opt-out

[vivshankar]

@aeneasr Just got to my machine. To substantiate further, we could minimally add a config parameter to control the instrumentation. I was also thinking for handlers to add a wrapper handler that basically does the same thing and it becomes a simple matter of adding that to your compose factory/handler list.

For custom code blocks, it is simple enough to add an interface for spans and transactions.

[aeneasr]

I don't think that we want to maintain an abstraction for telemetry in Fosite, especially as we are quite happy with otel at the moment.

If you don't have an otel compatible system, you can set the otel tracer to noop/nil in which case nothing happens. And you can still wrap Fosite any way you want to add instrumentation that works with vendor XYZ.

Being honest, I don't see the value of doing a lot of work here right now.