Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should instrumentations have dependencies to the libraries they instrument? #215

Closed
mariojonke opened this issue Nov 25, 2020 · 5 comments
Closed
Labels

Comments

@mariojonke
Copy link
Contributor

As of now instrumentations directly reference the library they instrument. This might not actually be required since an application which wants to instrument a library with a certain instrumentation should already have the library installed, otherwise it wouldn't be able to use it. The instrumentation then would only require dependency for running tests (via options.extras_require)

Directly Including the dependency in the instrumentation might actually cause problems where the application and the instrumentation are installed separatly, e.g. in an AWS Lambda function a library could be deployed in one layer with version x and the instrumentation in another layer with the library in version y. Without the dependency the library would only be included in the lib layer but instrumentation would still work.

@lzchen
Copy link
Contributor

lzchen commented Nov 25, 2020

Might be a stupid question but how would patching library specific methods work if the instrumentation doesn't take a dependency on the library? Like here?

@mariojonke
Copy link
Contributor Author

Since in python everything is dynamic this shouldn't be a problem. E.g. having an application which uses requests. This application already must have a dependency to requests as otherwise it won't be able to use it. So if one wants to instrument this application with the RequestsInstrumentor, the instrumentation package does not need to have the dependency to requests since it is already fulfilled by the application. Using an instrumentor without the application having a dependency to the library which is being instrumented does not make much sense since the library won't be used by anyone.

@lzchen
Copy link
Contributor

lzchen commented Nov 25, 2020

I don't like the idea of an instrumentation crashing on start up (even if this is not how it is "meant" to be used), it makes the software look "bad". I know it is obvious, but if we go this route, we have to enforce users to have these instrumented libraries installed or else the instrumentations will actually break. I would actually be fine with this if we can add checks to see if it is actually installed, and then patch the functions if so. This is contingent upon the use case listed above (with the different versions of instrumented libraries) to be an actual common problem.

@github-actions
Copy link

github-actions bot commented Apr 6, 2021

This issue was marked stale due to lack of activity. It will be closed in 30 days.

@srikanthccv
Copy link
Member

Addressed in #475

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants