Cherrypy instrumentation

[TheAnshul756]

trafficstars

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #1130

Type of change

Please delete options that are not relevant.

• [x] New feature (non-breaking change which adds functionality)
• [x] 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 A

Does This PR Require a Core Repo Change?

• [x] No.

Checklist:

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

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

[TheAnshul756]

I've came up with three different approaches for instrumentation in cherrypy and I wanted to discuss all three of them with there pros and cons.

Instrumentation with CherryPy Hooks

Unlike other frameworks, CherryPy does not have support for custom middleware. Instead, CherryPy provides some hook points where you can add your functions. Datadog uses this approach for instrumentation. But all the endpoints for which tools need to be used must be decorated by that tool. Eg.

class Root(object):
    @cherrypy.expose
    @cherrypy.tools.logit()
    def index(self):
        return "hello world"

Because of it, this approach can only be used for manual instrumentation, not auto-instrumentation. Although there are some workarounds that I came up with (like monkey patching the expose function) but they were also not covering all the possible cases (what if the function is not exposed with expose decorator index.exposed = true).

Instrumentation with CherryPy Publisher Subscriber

CherryPy’s backbone consists of a bus system implementing a simple publish/subscribe messaging pattern. Simply put, in CherryPy everything is controlled via that bus.

Various channels are available in CherryPy, but only two were helpful to us, i.e. before_request and after_request. Scout Python APM Agent use this approach of instrumentation. But the challenge with this approach is the before_request channel triggers at a very early stage of the request handling cycle, even before the request object parses the environ dictionary from the HTTP message, because of which we can not do much in before_request (including span creation). Thus this approach leaves us with a significant limitation.

Instrumentation by Patching CherryPy Application class

This is how we have instrumented various frameworks, and this approach can also be done here. New Relics and Appdynamics agent also uses this approach. The issue with this approach is Application class is implemented in _cptree module in CherryPy and imported in __init__ module. We can patch the class imported in __init__, but this will not enable instrumentation as CherryPy creates the object of the Application class in _cptree and uses the Application from that module directly, which forces us to instrument Application class both in __init__ and _cptree. I have used this approach for instrumenting in my PR but @srikanthccv believes

We can't use the private symbols because they are not guaranteed to remain backwards compatible.

I've implemented all three approaches and tested them to understand their pros and cons. The first two approaches will also come with other challenges, such as handling exceptions. And the third approach, although being backward incompatible, is tried and tested and guarantees to work. I need help knowing what to do next and which approach to proceed with. Also, if anyone has other ideas, I'm more than interested to know.

[srikanthccv]

This is how we have instrumented various frameworks, and this approach can also be done here.

I am not against the idea of patching the Application class but I don't think it is a good idea to patch the private/internals of the framework.

And the third approach, although being backward incompatible, is tried and tested and guarantees to work.

What is guaranteed here?

Overall, I don't think it's a good idea to patch the private/internals of the library. We have seen this fail with a few instrumentations, so they either had to be pinned to some minor version which works or rewritten to not patch private symbols and get it to work in general. I would like to hear others thoughts on this @open-telemetry/opentelemetry-python-contrib-approvers .

[TheAnshul756]

What is guaranteed here?

Sorry for the misunderstanding. What I mean by 'guarantee' here is that with other approaches, some loopholes could be there with instrumentation, like some exceptions could be missed or the way the endpoints are exposed could affect the instrumentation. And this approach covers all the possible cases.

I am not against the idea of patching the Application class but I don't think it is a good idea to patch the private/internals of the framework.

I understand your concern, which is why I tried instrumenting with other approaches, but nothing seems to work completely unless someone has a better idea.

[sanketmehta28]

Also resolve the build checks

[TheAnshul756]

Overall, I don't think it's a good idea to patch the private/internals of the library. We have seen this fail with a few instrumentations, so they either had to be pinned to some minor version which works or rewritten to not patch private symbols and get it to work in general. I would like to hear others thoughts on this.

@open-telemetry/opentelemetry-python-contrib-approvers Please share your thoughts on this.