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

SQLAlchemy Instrumentation: enable_commenter can't be disabled. #1368

Closed
tveastman opened this issue Oct 3, 2022 · 3 comments · Fixed by #1440
Closed

SQLAlchemy Instrumentation: enable_commenter can't be disabled. #1368

tveastman opened this issue Oct 3, 2022 · 3 comments · Fixed by #1440
Assignees
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed instrumentation

Comments

@tveastman
Copy link

Based on the documentation, you'd assume that:

SQLAlchemyInstrumentor().instrument(enable_commenter=False)

Would enable instrumentation but disable the statement commenter, but it doesn't work. It looks like you can't disable the sql commenter unless you are providing the engine keyword as well, which requires you to have already created the engine at that point in your code path.

It looks like the enable_commenter and related kwargs need to be passed to the _wrap_create_engine function?

@tveastman tveastman added the bug Something isn't working label Oct 3, 2022
@srikanthccv
Copy link
Member

It looks like the enable_commenter and related kwargs need to be passed to the _wrap_create_engine function?

Correct, and pass it to EngineTracer which enables commenter by default.

@srikanthccv srikanthccv added good first issue Good for newcomers help wanted Extra attention is needed instrumentation labels Nov 5, 2022
@avzis
Copy link
Contributor

avzis commented Nov 6, 2022

hi, can you assign this to me? thanks :)

@avzis
Copy link
Contributor

avzis commented Nov 24, 2022

I did the change to pass enable_commenter to EngineTracer.

But the problem can still happen, because the functionality of enable_commenter depends on calling EngineTracer.

If you initiate the instrumentation using an existing engine, the instrument(engine) will wrap the engine with EngineTracer
But if you initiate the instrumentation without an engine, and create an engine later on, you will have to wrap that engine manually in order to enable/disable the enable_commenter like this:

        SQLAlchemyInstrumentor().instrument()
        engine = create_engine("sqlite:///:memory:")
        EngineTracer(
            _get_tracer(self.tracer_provider), engine, enable_commenter=True
        )

This basically means that you either have to initiate the instrumentation using an existing engine, or manually call EngineTracer

I would like to hear opinions on how to solve this..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed instrumentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants