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

docs(instrumentation-redis): add instrumentation options #1029

Merged
merged 1 commit into from
May 24, 2022

Conversation

unflxw
Copy link
Contributor

@unflxw unflxw commented May 19, 2022

Which problem is this PR solving?

The Redis instrumentation already accepts several options on initialisation, in a similar manner to the IORedis instrumentation. Unlike in the IORedis instrumentation, however, these options are undocumented for the Redis instrumentation. This PR copies the relevant documentation about initialisation options from the IORedis instrumentation's README to the Redis one, tweaking it where the instrumentation libraries differ.

Short description of the changes

  • Copy documentation on IORedis instrumentation's initialisation options to the Redis instrumentation's README.
  • Do not copy any documentation relating to the requestHook option, as it is not supported by the Redis instrumentation.
  • Amend the documentation to document differences in default values:
    • In the IORedis instrumentation, the default db.statement serialiser returns the command name and arguments concatenated by spaces. The Redis instrumentation returns only the command name. The examples given for custom db.statement serialiser functions on each library's README correspond to the other library's default db.statement serialiser.
    • In the IORedis instrumentation, the default value for requireParentSpan is true. In the Redis instrumentation, it is false.

@unflxw unflxw requested a review from a team May 19, 2022 11:40
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 19, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: unflxw / name: Noemi (fed7e38)

@unflxw unflxw force-pushed the document-redis-options branch from c9b90ac to fed7e38 Compare May 19, 2022 12:24
@codecov
Copy link

codecov bot commented May 24, 2022

Codecov Report

Merging #1029 (fed7e38) into main (116caab) will decrease coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1029      +/-   ##
==========================================
- Coverage   95.91%   95.73%   -0.18%     
==========================================
  Files          13       16       +3     
  Lines         856     1008     +152     
  Branches      178      198      +20     
==========================================
+ Hits          821      965     +144     
- Misses         35       43       +8     
Impacted Files Coverage Δ
...tapackages/auto-instrumentations-node/src/utils.ts 97.91% <0.00%> (ø)
...metry-instrumentation-redis/src/instrumentation.ts 93.47% <0.00%> (ø)
...e/opentelemetry-instrumentation-redis/src/utils.ts 93.10% <0.00%> (ø)

@blumamir blumamir merged commit 4b36b75 into open-telemetry:main May 24, 2022
@unflxw unflxw deleted the document-redis-options branch May 24, 2022 09:19
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