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

Modify MassTransit Instrumentation to take advantage of sampling decision #94

Conversation

utpilla
Copy link
Contributor

@utpilla utpilla commented Mar 17, 2021

Extends the optimization done in the PR #1894 to optimize MassTransit Instrumentation

Changes

  • Added a check to return from the OnStartActivity method of the listener if the instrumentation is suppressed
  • Wrap the logic in OnStartActivity to perform operations such as filter, set the activity DisplayName, etc. only when activity.IsAllDataRequested is set to true
  • Removed the redundant filtering check in the OnStopActivity method of the listener
  • Added unit tests to check if the instrumentation respects Sdk.SuppressInstrumentation and activity.IsAllDataRequested

@utpilla utpilla requested a review from a team March 17, 2021 19:40
@codecov
Copy link

codecov bot commented Mar 17, 2021

Codecov Report

Merging #94 (91ef40e) into main (cbae2b5) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 91ef40e differs from pull request most recent head 68c0222. Consider uploading reports for the commit 68c0222 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main      #94      +/-   ##
==========================================
- Coverage   77.52%   77.51%   -0.02%     
==========================================
  Files          52       52              
  Lines        1455     1454       -1     
==========================================
- Hits         1128     1127       -1     
  Misses        327      327              
Impacted Files Coverage Δ
...it/Implementation/MassTransitDiagnosticListener.cs 93.90% <100.00%> (-0.08%) ⬇️

@cijothomas
Copy link
Member

@alexvaluyskiy Please review.

@cijothomas
Copy link
Member

@alexvaluyskiy Merging this now. Please share if you have any additional feedback.

@cijothomas cijothomas merged commit 937ec6f into open-telemetry:main Mar 19, 2021
@utpilla utpilla deleted the utpilla/Optimize-MassTransit-Instrumentation-For-SuppressInstrumentation branch May 26, 2023 00:28
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