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

Instrumentation for asyncpg #814

Merged
merged 16 commits into from
Jun 17, 2020

Conversation

HiveTraum
Copy link
Contributor

Hi everyone!

Asyncpg instrumentation. Tested on versions 0.12.0 and 0.20.1

@HiveTraum HiveTraum requested a review from a team June 11, 2020 11:06
Copy link
Member

@cnnradams cnnradams left a comment

Choose a reason for hiding this comment

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

Looks pretty good overall, some comments from a first pass:

Copy link
Member

@cnnradams cnnradams left a comment

Choose a reason for hiding this comment

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

LGTM, a couple non-blocking comments

ext/opentelemetry-ext-asyncpg/setup.cfg Outdated Show resolved Hide resolved
tox.ini Show resolved Hide resolved
Copy link
Contributor

@lzchen lzchen left a comment

Choose a reason for hiding this comment

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

LGTM. Small comment about tox.

@HiveTraum HiveTraum requested a review from cnnradams June 17, 2020 03:53
Copy link
Member

@toumorokoshi toumorokoshi left a comment

Choose a reason for hiding this comment

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

LGTM! A few small changes.

ext/opentelemetry-ext-asyncpg/setup.cfg Outdated Show resolved Hide resolved
ext/opentelemetry-ext-asyncpg/setup.cfg Outdated Show resolved Hide resolved
ext/opentelemetry-ext-asyncpg/README.rst Outdated Show resolved Hide resolved
@toumorokoshi toumorokoshi merged commit a7f0b75 into open-telemetry:master Jun 17, 2020
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.

4 participants