Skip to content

feat: implement on ending in span processor#6024

Merged
pichlermarc merged 1 commit intoopen-telemetry:mainfrom
majanjua-amzn:main
Nov 13, 2025
Merged

feat: implement on ending in span processor#6024
pichlermarc merged 1 commit intoopen-telemetry:mainfrom
majanjua-amzn:main

Conversation

@majanjua-amzn
Copy link
Copy Markdown
Contributor

@majanjua-amzn majanjua-amzn commented Oct 17, 2025

Which problem is this PR solving?

Implementation of in-development feature onEnding of the opentelemetry specification. This will allow users to modify spans before they become immutable as they are in the process of being ended

Fixes #5836

Short description of the changes

  • Implementation of onEnding as optional and @experimental
  • Added onEnding to MultiSpanProcessor
  • Added unit tests to test onStart, onEnding, and onStart are called in the correct order

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • Unit Testing

Checklist:

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

@majanjua-amzn majanjua-amzn requested a review from a team as a code owner October 17, 2025 22:33
JacksonWeber
JacksonWeber approved these changes Oct 21, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@JacksonWeber JacksonWeber left a comment

Choose a reason for hiding this comment

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

Generally, LGTM, but it looks like the workflows need kicked off again to properly run the tests.

@majanjua-amzn
Copy link
Copy Markdown
Contributor Author

majanjua-amzn commented Oct 22, 2025

Generally, LGTM, but it looks like the workflows need kicked off again to properly run the tests.

How can I get this done? I don't have access to approve the GitHub workflows.

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 24, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.16%. Comparing base (ded89b5) to head (7469968).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #6024   +/-   ##
=======================================
  Coverage   95.16%   95.16%           
=======================================
  Files         316      316           
  Lines        9220     9226    +6     
  Branches     2080     2082    +2     
=======================================
+ Hits         8774     8780    +6     
  Misses        446      446
Files with missing lines Coverage Δ
...telemetry-sdk-trace-base/src/MultiSpanProcessor.ts 100.00% <100.00%> (ø)
packages/opentelemetry-sdk-trace-base/src/Span.ts 97.70% <100.00%> (+0.02%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@maryliag
Copy link
Copy Markdown
Contributor

maryliag commented Oct 24, 2025

hey @majanjua-amzn, you're missing an entry on the CHANGELOG file :)

@majanjua-amzn majanjua-amzn force-pushed the main branch 3 times, most recently from fc5c12d to 26a0b17 Compare October 24, 2025 16:22
david-luna
david-luna requested changes Nov 12, 2025
View reviewed changes
Comment thread packages/opentelemetry-sdk-trace-base/src/Span.ts
pichlermarc
pichlermarc requested changes Nov 12, 2025
View reviewed changes
Copy link
Copy Markdown
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.

thanks for working on this. once the order is aligned with the spec we can merge this in.

Comment thread CHANGELOG.md
Comment thread packages/opentelemetry-sdk-trace-base/src/Span.ts
Comment thread packages/opentelemetry-sdk-trace-base/src/Span.ts
Comment thread packages/opentelemetry-sdk-trace-base/src/SpanProcessor.ts Outdated
david-luna
david-luna approved these changes Nov 13, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@david-luna david-luna left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@majanjua-amzn thank you very much for your contribution :)

@pichlermarc I think is ready for your review

@pichlermarc pichlermarc added this pull request to the merge queue Nov 13, 2025
Merged via the queue into open-telemetry:main with commit a05e581 Nov 13, 2025
27 checks passed
@otelbot
Copy link
Copy Markdown
Contributor

otelbot bot commented Nov 13, 2025

Thank you for your contribution @majanjua-amzn! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

@otelbot-js otelbot-js bot mentioned this pull request Jan 7, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@david-luna david-luna david-luna approved these changes

@pichlermarc pichlermarc pichlermarc approved these changes

@JacksonWeber JacksonWeber JacksonWeber approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[sdk-trace-*] implement experimental SpanProcessor#onEnding(Span)

5 participants

@majanjua-amzn @maryliag @david-luna @pichlermarc @JacksonWeber