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

fix(memcached): low cardinality span name #1104

Merged
merged 2 commits into from
Aug 10, 2022

Conversation

aadharsh-rengarajan
Copy link
Contributor

@aadharsh-rengarajan aadharsh-rengarajan commented Aug 1, 2022

Which problem is this PR solving?

We have addressed the problem above by making the changes necessary and we can get the memcached value from the span attributes itself.

Short description of the changes

  • Removed query.type from the span name as it creates a high cardinality of span names.
  • The span name will be memcached get or memcached set. (Open to change this span name)

Checklist

  • Ran npm test for the edited package(s) on the latest commit if applicable.

@aadharsh-rengarajan aadharsh-rengarajan requested a review from a team August 1, 2022 14:16
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 1, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: aadharsh-rengarajan / name: aadharsh-rengarajan (fe86375)

@github-actions github-actions bot requested a review from rauno56 August 1, 2022 14:16
@aadharsh-rengarajan aadharsh-rengarajan changed the title Clean span name in memcached instrumentation fix: clean span name in memcached instrumentation Aug 1, 2022
@aadharsh-rengarajan aadharsh-rengarajan force-pushed the span-name-change branch 2 times, most recently from 4d37bc6 to cbe323e Compare August 1, 2022 14:40
@codecov
Copy link

codecov bot commented Aug 2, 2022

Codecov Report

Merging #1104 (67d43f7) into main (4d62c92) will decrease coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1104      +/-   ##
==========================================
- Coverage   96.07%   95.95%   -0.12%     
==========================================
  Files          14       17       +3     
  Lines         892     1014     +122     
  Branches      191      210      +19     
==========================================
+ Hits          857      973     +116     
- Misses         35       41       +6     
Impacted Files Coverage Δ
...y-instrumentation-memcached/src/instrumentation.ts 96.42% <100.00%> (ø)
...tapackages/auto-instrumentations-node/src/utils.ts 98.00% <0.00%> (ø)
...entelemetry-instrumentation-memcached/src/utils.ts 81.25% <0.00%> (ø)

@blumamir blumamir changed the title fix: clean span name in memcached instrumentation fix(memcached): low cardinality span name Aug 10, 2022
@dyladan dyladan merged commit cff4e77 into open-telemetry:main Aug 10, 2022
@dyladan dyladan mentioned this pull request Aug 10, 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.

Options to set only span key in span name of memcached instrumentation
5 participants