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

feat: allow adding links after span creation #4536

Merged
merged 21 commits into from May 6, 2024

Conversation

seemk
Copy link
Contributor

@seemk seemk commented Mar 11, 2024

Which problem is this PR solving?

Fixes #4476

Short description of the changes

Adds functions to add span links after span creation.

Additionally deleted the test named should set a link as it did not test anything and was a duplicate of should return ReadableSpan with links.

Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Existing users implementing the span API will see compilation breakage.

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added

@seemk seemk requested a review from a team as a code owner March 11, 2024 10:29
Copy link

codecov bot commented Mar 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.83%. Comparing base (2610122) to head (fc9acbc).
Report is 19 commits behind head on main.

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4536      +/-   ##
==========================================
+ Coverage   90.77%   92.83%   +2.05%     
==========================================
  Files          90      328     +238     
  Lines        1930     9529    +7599     
  Branches      417     2050    +1633     
==========================================
+ Hits         1752     8846    +7094     
- Misses        178      683     +505     
Files Coverage Δ
api/src/trace/NonRecordingSpan.ts 95.65% <100.00%> (+0.91%) ⬆️
packages/opentelemetry-sdk-trace-base/src/Span.ts 97.59% <100.00%> (+0.09%) ⬆️

... and 244 files with indirect coverage changes

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Looks good % changelog nits.
Thanks for working on this. 🙂

I will merge this in when all batched with metrics API changes for the next release (and when all changes-requested reviews are resolved)

api/CHANGELOG.md Outdated Show resolved Hide resolved
api/CHANGELOG.md Outdated Show resolved Hide resolved
@pichlermarc
Copy link
Member

@martinkuba friendly ping. 🙂 Is this still blocked from your point of view?

@seemk
Copy link
Contributor Author

seemk commented Apr 25, 2024

@martinkuba

@pichlermarc pichlermarc merged commit c503ff1 into open-telemetry:main May 6, 2024
19 checks passed
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.

[api][sdk-trace] implement Span.addLink()
3 participants