Skip to content

Conversation

@OlegZv
Copy link
Contributor

@OlegZv OlegZv commented Dec 22, 2024

Description

This PR allows instrumenting only a specific Redis client instead of the whole class (and all clients).
Changes:

  • ability to instrument a specific Redis client with instrument_client (supports any of the existing clients such as async/cluster)
  • Added more typing for instrument and the newly added instrument_client methods (still allows **kwargs). Makes the docs easier to read/understand.
  • Moved some of the if condition checks to global "defines" to make the code a bit more readable.
  • Updated the docs for a more consistent structure (add specific sections).
  • Added Redis sphinx reference so the docs can correctly link up agains Redis clients.

Please delete options that are not relevant.

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

How Has This Been Tested?

Ran the already included tests with python3.12 and python3.11 and redis 5.0.1. Added more tests to ensure that only the client that we instrumented produces spans. Ran tox tests, ruff, linting, formatting, etc. (everythin in the contributing guide).

Tested docs changes with tox -e docs to ensure everything builds correctly and looks good.
Also, ran tests with coverage: original coverage was 77% now it's 80%:
image

Does This PR Require a Core Repo Change?

  • No.

Checklist:

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

  • Followed the style guidelines of this project
  • Changelogs have been updated (will update after opening this PR)
  • Unit tests have been added
  • Documentation has been updated

@OlegZv OlegZv requested a review from a team as a code owner December 22, 2024 04:13
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 22, 2024

CLA Missing ID CLA Not Signed

@OlegZv OlegZv closed this Dec 22, 2024
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.

2 participants