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

[WIP]: SQLAlchemy & psycopg2 Flask Integration #1138

Closed

Conversation

Thiyagu55
Copy link
Contributor

@Thiyagu55 Thiyagu55 commented Jun 17, 2022

SQLAlchemy & Psycopg2 SQLCommenter support for Flask related metrics

Support for using flask related metrics in sqlalchemy & psycopg2 SQL commenter.

This can be achieved by enabling enable_commenter flag inside Flask instrument initialization and also in SQLAlchemy Instrumentation or psycopg2 instrumentation

FlaskInstrumentor().instrument_app(app, tracer_provider=provider, enable_commenter=True)

# For sqlalchemy
SQLAlchemyInstrumentor().instrument( enable_commenter=True, engine=engine)

# For psycopg2
Psycopg2Instrumentor().instrument(tracer_provider=tracer_provider, enable_commenter=True)
  • The flask instrumentation will append the metrics in opentelemetry context in key name SQLCOMMENTER_VALUES
  • The SQLAlchemy ORM or psycopg2 driver will read the context and then append these info to the sql query logs whenever the sql query is made

@srikanthccv Need your suggestions for this draft PR so we could implement in full scale

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

Tests needs to be added upon approval of this solution

  • Test A

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • 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

@Thiyagu55 Thiyagu55 requested a review from a team as a code owner June 17, 2022 12:17
@Thiyagu55 Thiyagu55 marked this pull request as draft June 17, 2022 12:18
@srikanthccv
Copy link
Member

@Thiyagu55 this is probably not the best way to go about it. We can't do this for all instrumentation. Maybe we should make use of the context? The sqlcommenter will the read the request meta from the current context and frameworks can independently set meta in context.

@Thiyagu55
Copy link
Contributor Author

@Thiyagu55 this is probably not the best way to go about it. We can't do this for all instrumentation. Maybe we should make use of the context? The sqlcommenter will the read the request meta from the current context and frameworks can independently set meta in context.

@srikanthccv Updated the code so that the interchange takes place via context. Could you verify?

@srikanthccv
Copy link
Member

@Thiyagu55 I would like to see what other approvers/maintainers think of this approach first and see if they have any other ideas.

@Thiyagu55
Copy link
Contributor Author

@Thiyagu55 I would like to see what other approvers/maintainers think of this approach first and see if they have any other ideas.

Sure

@Thiyagu55 Thiyagu55 changed the title [WIP]: SQLAlchemy Flask Integration [WIP]: SQLAlchemy & psycopg2 Flask Integration Jul 4, 2022
@Thiyagu55
Copy link
Contributor Author

@Thiyagu55 I would like to see what other approvers/maintainers think of this approach first and see if they have any other ideas.

Sure

@srikanthccv Can you tag other reviewers for their opinion on this approach?

@srikanthccv
Copy link
Member

@Thiyagu55 Sorry, haven't been able to bring this up in SIG because some approver/maintainers were OOO. Would you be able to join SIG meeting this week?

@Thiyagu55
Copy link
Contributor Author

@Thiyagu55 Sorry, haven't been able to bring this up in SIG because some approver/maintainers were OOO. Would you be able to join SIG meeting this week?

@sjs994 FYA

@Thiyagu55
Copy link
Contributor Author

Thiyagu55 commented Jul 21, 2022

@Thiyagu55 Sorry, haven't been able to bring this up in SIG because some approver/maintainers were OOO. Would you be able to join SIG meeting this week?

@sjs994 FYA

@srikanthccv Subhrajyoti and me will join the meeting tonight

@Thiyagu55
Copy link
Contributor Author

@Thiyagu55 Thiyagu55 closed this Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants