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

Redis: Adding the ability to disable adding Events to Activities #416

Merged
merged 7 commits into from
Jun 24, 2022

Conversation

stebet
Copy link
Contributor

@stebet stebet commented Jun 10, 2022

Changes

Adds an option for OpenTelemetry.Instrumentation.StackExchangeRedis to disable adding the Redis command lifetime ActivityEvents to Activities. Added this because for example in Honeycomb, Span Events are billed and measured just like individual spans, so Activity Events can quickly gobble up telemetry budgets.

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #

@stebet stebet requested a review from a team as a code owner June 10, 2022 09:49
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 10, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: stebet / name: Stefán Jökull Sigurðarson (d141ec2, 2813a31)

@stebet
Copy link
Contributor Author

stebet commented Jun 10, 2022

Another option might be to convert these Events into Tags instead, in case users do not want a separate event for these but would still like the info to be part of the span.

@utpilla utpilla added the comp:instrumentation.stackexchangeredis Things related to OpenTelemetry.Instrumentation.StackExchangeRedis label Jun 10, 2022
@codecov
Copy link

codecov bot commented Jun 17, 2022

Codecov Report

Merging #416 (bd077fe) into main (76f0b06) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #416   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        163     163           
  Lines       4907    4909    +2     
=====================================
- Misses      4907    4909    +2     
Impacted Files Coverage Δ
...mentation/RedisProfilerEntryToActivityConverter.cs 0.00% <0.00%> (ø)
...s/StackExchangeRedisCallsInstrumentationOptions.cs 0.00% <0.00%> (ø)

@ejsmith
Copy link
Contributor

ejsmith commented Jun 23, 2022

This is exactly what I was about to create a PR for. Thank you @stebet! In my case, I'm using Elasticsearch APM and it is converting these activity events into log messages and flooding my app logs with literally millions and millions of useless log messages:

image

@ejsmith
Copy link
Contributor

ejsmith commented Jun 23, 2022

It would be super appreciated if we could get this PR merged and a new nuget package pushed out because this behavior is killing my ES APM logs.

@cijothomas
Copy link
Member

we can merge once CI is green. @stebet could you fix the markdownlint issues?

@cijothomas cijothomas merged commit e252892 into open-telemetry:main Jun 24, 2022
@ejsmith
Copy link
Contributor

ejsmith commented Jun 24, 2022

Awesome! Can we get a new release pushed out? Also, while we are at it, can we do a new release for the Elasticsearch package as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.stackexchangeredis Things related to OpenTelemetry.Instrumentation.StackExchangeRedis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants