Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: avoid ending spans multiple times #400

Merged
merged 2 commits into from
Apr 10, 2023

Conversation

seemk
Copy link
Contributor

@seemk seemk commented Apr 10, 2023

#379 changed the existing code such that ending a span multiple times sends it to the processor/exporter multiple times. With the previous logic this did not happen.

Added a quick bailout when ending a span and it has already ended.

@codecov
Copy link

codecov bot commented Apr 10, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (2d7f897) 62.37% compared to head (c0c8292) 62.39%.

❗ Current head c0c8292 differs from pull request most recent head 6969ff9. Consider uploading reports for the commit 6969ff9 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #400      +/-   ##
==========================================
+ Coverage   62.37%   62.39%   +0.01%     
==========================================
  Files         253      253              
  Lines       11765    11770       +5     
==========================================
+ Hits         7339     7344       +5     
  Misses       4426     4426              
Impacted Files Coverage Δ
...nTelemetrySdk/Trace/RecordEventsReadableSpan.swift 96.05% <100.00%> (+0.09%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@nachoBonafonte nachoBonafonte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yes, good catch, the previous code was just exiting the block instead of the function, my bad

@nachoBonafonte
Copy link
Member

Thanks a lot for your contribution

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants