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

Tortoise ORM instrumentation #685

Merged
merged 27 commits into from
Nov 22, 2022
Merged

Conversation

tonybaloney
Copy link
Contributor

@tonybaloney tonybaloney commented Sep 19, 2021

Description

This PR introduces an instrumentation module for Tortoise ORM https://tortoise-orm.readthedocs.io/en/latest/getting_started.html

Type of change

Please delete options that are not relevant.

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

  • Implemented an instrumented application using Tortoise with a console exporter

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

@tonybaloney tonybaloney requested a review from a team September 19, 2021 01:35
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 19, 2021

CLA Signed

The committers are authorized under a signed CLA.

@tonybaloney
Copy link
Contributor Author

Requested CLA from Microsoft signatory.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! Please add a note to the changelog about the new integration and unit tests.

@lzchen
Copy link
Contributor

lzchen commented Sep 20, 2021

@tonybaloney

Following the expectations from contributors, would it be alright if you were made a CODEOWNER for this package?

@lzchen
Copy link
Contributor

lzchen commented Sep 29, 2021

@tonybaloney
Be sure to add tortoise instrumentation to bootstrap_gen.py to fix the generate build.
Similar to open-telemetry/opentelemetry-python#2160

ocelotl
ocelotl previously approved these changes Oct 20, 2021
@titogarrido
Copy link

Hi guys! this change is not merged, right?

@srikanthccv
Copy link
Member

I think we forgot to merge this. It has enough approvals. If the tests pass, I will merge this; otherwise, they must be fixed first.

@ocelotl ocelotl force-pushed the tortoise branch 2 times, most recently from eb0d997 to 61c2283 Compare November 18, 2022 11:01
@ocelotl ocelotl dismissed their stale review November 18, 2022 14:01

I added some commits while updating this branch.

@ocelotl
Copy link
Contributor

ocelotl commented Nov 18, 2022

@open-telemetry/python-approvers PTAL, big disclaimer: this instrumentation has no functional test cases. I added one just to make pytest pass. I guess we can merge as it is (since it is not something that would block an user) but wanted to let you know.

@srikanthccv
Copy link
Member

This has enough and can be merged 👍 .

@lzchen lzchen enabled auto-merge (squash) November 22, 2022 02:08
@lzchen lzchen merged commit b6b2690 into open-telemetry:main Nov 22, 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.

7 participants