Remove Configuration from instrumentations

[ocelotl]

Description

As discussed here we are removing Configuration from the core repo. This means that it needs to be removed here as well. This PR does that.

This also adds an opentelemetry.util namespace where util code can be added to. Should this go into a separate PR? I also think so. Nevertheless, I am adding this here because there was some non-configuration-related code in the old Configuration class that I am moving into this namespace now. I understand that this can be controversial, so if you prefer this to be split into 2 PRs comment below, please. 👍

Fixes #284

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Does This PR Require a Core Repo Change?

• Yes. - Link to PR: Remove Configuration opentelemetry-python#1523
• 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

[lzchen]

Please update the title and description of the PR to be more informative.

[codeboten]

PR looks pretty good, just waiting on responses to the comment about moving the excludelist code into the instrumentation package before approving.

[ocelotl]

Please update the title and description of the PR to be more informative.

Done! 👍

[srikanthccv]

Overall LGTM except the decision to move WSGI/ASGI middleware instrumentation libraries to util.

[srikanthccv]

I believe this wsgi instrumentation library could be used for any wsgi framework to collect telemetry. For instance let's say I have a WSGI based framework X and there is no standalone instrumentation for it. If we remove this what would be the way for collecting telemetry?

[ocelotl]

The WSGI code as it is now is not an instrumentation because it does no implement BaseInstrumentor. There is no WSGI library that is instrumented by it as opposed to specific libraries like Flask, Django, etc. For that reason, the WSGI code should not be in opentelemetry.instrumentation.

I don't understand what you mean with "For instance let's say I have a WSGI based framework X and there is no standalone instrumentation for it. If we remove this what would be the way for collecting telemetry?", can you explain, please?

[srikanthccv]

The WSGI and ASGI are different instrumentation libraries on their own as of now. I may be wrong but my understanding is that I can use opentelemetry-instrumentation-wsgi with any wsgi based web frameworks to collect telemetry. I will try to use an example to clarify the last part, We don't have the instrumentation library for cherrpy framework but I can still instrument my cherrpy based application using opentelemetry-instrumentation-wsgi. It would look something similar as followed

import cherrypy
from opentelemetry.instrumentation.wsgi import OpenTelemetryMiddleware

class HelloWorld(object):
    @cherrypy.expose
    def index(self):
        return "Hello World!"

app  = cherrypy.Application(HelloWorld(), '/')

app.wsgiapp = OpenTelemetryMiddleware(app.wsgiapp)

cherrypy.quickstart(app)

I didn't think of WSGI/ASGI OpenTelemetryMiddleware as an util although there are some util functions in that package. It was easier to find that library, docs and use because it is very similar to any other popular middlewares flask-cache or bottle middlewares etc.. to name few. I thought may be util is not an ideal place to put middleware kind of stuff, just wanted to understand the motivation behind moving from individual packages to util. On the other hand I think it is good to move the helper functions under util.http.

[mariojonke]

Not sure about moving the WSGI/ASGI OpenTelemetryMiddleware classes to the opentelemetry-util-http package. With this there are now 3 version.py files in the package since both OpenTelemetryMiddleware implementations are de facto instrumentations which require the version to get the tracer for producing spans.

Imo WSGI/ASGI OpenTelemetryMiddleware should stay in their own dedicated packages (common helper methods for extracting span attriubtes, ... can be moved to the util package though), also for dependency reasons. E.g. if one wants instrument an arbitrary WSGI application with the WSGI OpenTelemetryMiddleware from the opentelemetry-util-http the asgiref library will be pulled additionally even though it is not required.

[ocelotl]

@mariojonke @lonewolf3739

I understand that the current ASGI and WSGI packages are closely related to instrumentation. Nevertheless, they are not complete instrumentations as the rest of the instrumentations because they are not instrumenting anything by themselves when auto instrumentation happens nor they implement BaseInstrumentor. For that reason I believe they should not be part of any instrumentation package. The idea of util is pretty much to have a place where we can put anything that does not fit in the category of exporter, instrumentation, api, sdk, etc. I believe this to be the case, even when I see how this approach causes some concern because of the close relationship between the ASGI and WSGI and the instrumentation of other third party libraries.

Maybe having a util.instrumentation.asgi/wsgi package would be a better solution? 🤷

[srikanthccv]

Those are valid points for why asgi/wsgi middleware aren't really a otel instumentation library. How does the versioning and packaging happen? Isn't it like util lib as a whole bumps version even when there aren't any changes in middleware?

Maybe having a util.instrumentation.asgi/wsgi package would be a better

+1. Probably we should have good documentation to make it is easier to find and use these middleware as a substitute when there is no stand alone instrumentation for third party ASGI/WSGI frameworks.

[codeboten]

For the sake of getting the configuration piece moved forward, please keep the asgi/wsgi packages as separate for now. I feel like that discussion can be had separately from removing the config.

[ocelotl]

For the sake of getting the configuration piece moved forward, please keep the asgi/wsgi packages as separate for now. I feel like that discussion can be had separately from removing the config.

Sure, separating this

[codeboten]

Thanks for addressing my comments!