Add pymssql instrumentation

[guillaumep]

Description

This pull request implements instrumentation for the pymssql library (https://pypi.org/project/pymssql/). It is basically a copy of the pymysql instrumentation.

Still a work-in-progress.

Type of change

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

How Has This Been Tested?

• [x] Ran tox tests
• [x] Tested manually in a python program - verified that traces are properly sent

Does This PR Require a Core Repo Change?

• [ ] Yes. - Link to PR:
• [x] No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

• [x] Followed the style guidelines of this project
• [x] Changelogs have been updated [PARTIAL] -> I can complete that once the PR is opened and I have an issue number
• [x] Unit tests have been added
• [x] Documentation has been updated

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: guillaumep (62fbf8430fbb71b2ecb483a9cd5090bcb5fc3681, 2450145e77b0178f5aa7516eec19a16a2cf079a7)
• :white_check_mark: login: xrmx / name: Riccardo Magliocchetti (fe244a09b90913a458aa48e58b566755b2b8f79b)

[guillaumep]

It seems from the CI logs that pymssql does not build for pypy3.

Would it be acceptable to disable pypy3 support in the tests for this instrumentation?

[guillaumep]

Also, the docs fails to build because of this error:

sphinx.errors.SphinxWarning: autodoc: failed to import module 'pymssql' from module 'opentelemetry.instrumentation'; the following exception was raised: No module named 'pymssql'

I'm trying to figure out my error, can someone experienced with the repo help?

[guillaumep]

I had to subclass DatabaseApiIntegration to obtain pymssql's connection attributes, as the attributes are not kept by the library in the Connection object. You need to get them from the connect function parameter.

[linux-foundation-easycla[bot]]

The committers are authorized under a signed CLA.

• :white_check_mark: guillaumep (b93ad00ff2400793502e5994d95ff57090f5ed85)

[lzchen]

@guillaumep Are you still working on this?

[guillaumep]

@lzchen I was expecting help with the issues I mentioned in the comments above, but since I never got any replies up to now I had put this work aside.

pymssql instrumention is still a feature we want at work (outbox.com), so if I can have proper support regarding my questions I'll be glad to finish this PR.

Let me rebase the PR this week and post back here with the current issues I will have.

[lzchen]

@guillaumep Apologies for that. Usually PRs are not reviewed unless if marked as review (changed to an actual PR). If you would like help with your build issues, feel free to rebase and we can get you the assistance you need.

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: guillaumep (6cabf98d5cccdec346b438f19a62d9b128bb5477)

[guillaumep]

@lzchen now ready for review!

[lzchen]

@guillaumep Thanks for the contribution. Would you be willing to add yourself as an owner for this component under here?

[lzchen]

Please add an entry to the README

[guillaumep]

@lzchen currently working on the comments. Would you prefer I rebase, modify my current commit and force push on my branche, or would you rather like to have a new commit that addresses the comments?

[guillaumep]

@lzchen All comments addressed.

[lzchen]

Changes look good. Just a couple of outstanding comments. Also please fix the builds :)

[srikanthccv]

@guillaumep we are almost near to getting this merged. Please fix the tests and address the outstanding comments.

[ChristianWeyer]

@guillaumep we are almost near to getting this merged. Please fix the tests and address the outstanding comments.

Is there any chance we see pymssql support anytime soon 🙈? .cc @guillaumep

[guillaumep]

@ChristianWeyer I had planned to continue working in this PR this week, but please know it is a lot of work, help might be needed at some point!

[guillaumep]

Still to be done:

• [x] update instrumentation/pymssql/__init__.py according to the latest changes
• [x] make sure that PR tests work (the tests that run automatically seems to work)
• [ ] see if I need to update the docs (I've been told to update the doc but tox -e docs is not working even with Python 3.10)
• [x] actual testing with a local project -- works!!

[tammy-baylis-swi]

Thanks @guillaumep for resuming this!

To address one failure, please do this:

Generated workflows are out of date, run "tox -e generate-workflows" and commit the changes in this PR.

Might also need to update docs dependencies for another failure:

sphinx.errors.SphinxWarning: autodoc: failed to import module 'pymssql' from module 'opentelemetry.instrumentation'; the following exception was raised: No module named 'pymssql'

[guillaumep]

Generated workflows are out of date, run "tox -e generate-workflows" and commit the changes in this PR.

Done! However tox -e lint-instrumentation-pymssql fails with:

************* Module build.lib.opentelemetry.instrumentation.pymssql.__init__
opentelemetry-instrumentation-pymssql/build/lib/opentelemetry/instrumentation/pymssql/__init__.py:184:0: C0305: Trailing newlines (trailing-newlines)

But I am enable to fix the file, since there are no traling newlines in it... I've tried to add an ignore instruction but it did not fix the issue.

Might also need to update docs dependencies for another failure:

sphinx.errors.SphinxWarning: autodoc: failed to import module 'pymssql' from module 'opentelemetry.instrumentation'; the following exception was raised: No module named 'pymssql'

Fixed by adding to docs-requirements.txt.

[guillaumep]

@ChristianWeyer if you'd like you can take the PR, build the project and test it. It works for my own usage but additionnal feedback would be great and also help the maintainers merge this PR. After checking-out the PR:

pip install hatchling
cd instrumentation/opentelemetry-instrumentation-pymssql
# replace all occurences of `0.51b0.dev` to `0.50b0` in the folder, then:
hatchling build
pip install dist/opentelemetry_instrumentation_pymssql-0.50b0-py3-none-any.whl

[tammy-baylis-swi]

Thanks @guillaumep ! The automated checks are all passing now.

I just requested a bit more unit test coverage.

[ChristianWeyer]

@ChristianWeyer if you'd like you can take the PR, build the project and test it. It works for my own usage but additionnal feedback would be great and also help the maintainers merge this PR. After checking-out the PR:

pip install hatchling
cd instrumentation/opentelemetry-instrumentation-pymssql
# replace all occurences of `0.51b0.dev` to `0.50b0` in the folder, then:
hatchling build
pip install dist/opentelemetry_instrumentation_pymssql-0.50b0-py3-none-any.whl

Works!

Is there any way we can also see the result of the database interaction, e.g. query result?

[guillaumep]

Works!

Thanks for testing!

Is there any way we can also see the result of the database interaction, e.g. query result?

Not at the moment in this PR. There are also performance concerns about that feature (a lot of data could need to be sent to the tracing providers), and privacy concerns too if the data being queried concerns customers record for instance.

To my understanding of the project no other database instrumentation offer this feature neither. You may open a new issue to suggest the feature if you'd like.

[guillaumep]

I just requested a bit more unit test coverage.

Should be done, let me know if there's something missing before approval!

[guillaumep]

@xrmx @lzchen @aabmass Please have a look at this PR; could it be merged?

[lzchen]

@guillaumep Thanks for the contribution. Would you be willing to add yourself as an owner for this component under here?

Thanks for the patience on this review. Would you be able to add your name to component-owners.yml?

[guillaumep]

Thanks for the patience on this review. Would you be able to add your name to component-owners.yml?

For now yes.

[guillaumep]

@emdneto @lzchen I think I have resolved all items, but let me know.