Sqlalchemy uninstrument function doesnt work as expected

[liorzmetis]

Describe your environment Describe any aspect of your environment relevant to the problem, including your Python version: 3.9

Steps to reproduce
Just create a SQLAlchemy instrumentation and call uninstrument function
Example:

db_url = os.environ['DATABASE_URI']

engine = create_engine(db_url)

instrumentation = SQLAlchemyInstrumentor()
instrumentation.instrument(
    engine=engine
)

def set_interval(func, sec):
    def func_wrapper():
        # set_interval(func, sec)
        func()

    t = threading.Timer(sec, func_wrapper)
    t.start()
    return t


def instrument():
    print("instrument")
    instrumentation.instrument_app(app, engine)


def uninstrument():
    print("uninstrument")
    instrumentation.uninstrument()


set_interval(instrument, 1)
set_interval(uninstrument, 20)

What is the expected behavior?
Don't collect spans

What is the actual behavior?
Still recording and collecting spans

Additional context
Add any other context about the problem here.

[avzis]

I don't think this is still relevant.
validated by this PR: #1471

On a side note, you should call instrument() instead of instrument_app() when instrumenting

[srikanthccv]

@liorzmetis please share a fully reproducible example that still has the issue.

[shalevr]

I think I get the problem, The problem is with the Engine.
inside the Engine we call listen on events:

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py

Line 108 in c92ba14

listen(

But no one call remove on the events after the uninstrument
and for the uninstrument test, I created a new engine after uninstrument so the test missed this

I will fix this

[liorzmetis]

I think I get the problem, The problem is with the Engine. inside the Engine we call listen on events:

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py

Line 108 in c92ba14

listen(

But no one call remove on the events after the uninstrument
and for the uninstrument test, I created a new engine after uninstrument so the test missed this
I will fix this

Yeap, the engine still receiving events after uninstrument function has been called
In the mean time i fixed it on my project side by remove the event listener from the current engine:

    def _uninstrument(self, **kwargs):
        super()._uninstrument(**kwargs)
        engine = self.exporter_tracer.engine
        event.remove(engine, "before_cursor_execute", self.before_query_hook)
        event.remove(engine, "after_cursor_execute", _after_cur_exec)
        event.remove(engine, "handle_error", _handle_error)