feat: add withSpan helper

[Flarna]

Add withSpan helper to execute a function on a context holding a given span.

fixes: open-telemetry/opentelemetry-js#1923

I put the new API next to the existing helpers setSpan, getSpan,... as it seem to me the best match currently.
open-telemetry/opentelemetry-js#1750 discusses to use more namespacing but as of now this seems the best place form user point of view.

In open-telemetry/opentelemetry-js#1923 there were several proposals where to place it. I'm fine with moving it if consensus is on api.trace or api.context or ...

[codecov]

Codecov Report

Merging #7 (313a1f5) into main (53244ed) will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main       #7      +/-   ##
==========================================
+ Coverage   89.82%   89.95%   +0.13%     
==========================================
  Files          33       33              
  Lines         452      458       +6     
  Branches       77       77              
==========================================
+ Hits          406      412       +6     
  Misses         46       46              

Impacted Files	Coverage Δ
src/context/context.ts	93.47% <100.00%> (+0.97%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53244ed...299bb7a. Read the comment docs.

[obecny]

hmm I created an issue #1923 with a purpose to come to an agreement first and after this add it. But we shall all agreed for it beforehand not to duplicate our work, but here I see you just added something already, please assign yrself to a task first next time, thx.

[dyladan]

I think we had generally agreed on withSpan. Maybe not on other things but this basically what I had imagined.

[dyladan]

/cc @tedsuo

[obecny]

I think we had generally agreed on withSpan. Maybe not on other things but this basically what I had imagined.

Yes but it was added on context and we were discussing to have it directly on api

api.withSpan(....

[Flarna]

In open-telemetry/opentelemetry-js#1923 there was no final agreement. We had at least

• api.trace.withSpan
• api.context.withSpan
• api.withSpan

This PR is to nail this down. As indicated above I'm happy to move it if this is preferred. I decided to go for a PR instead an extra issue as the amount of work to add this is small but it has the advantage that people can try it and comment at the source itself.

Regarding the location. In general I'm in favor of namespaces therefore it would be api.context.withSpan. But I tried to get in consistent with the other APIs. Having it on context would lead to something like this which convinced me to move it to top level.

// withSpan on context but getSpan on api
api.context.withSpan(span, () => {
  // non context namespace here...
  const span = api.getSpan(context.active());
});

[vmarchaud]

I'm in favor of namespacing too so either api.context.withSpan or api.trace.withSpan but i have a preference for api.context.withSpan since those are just utils for the context api.

[obecny]

In open-telemetry/opentelemetry-js#1923 there was no final agreement.

Exactly, so I don't understand why did you create a PR and just chose one.

The main idea of adding helper is to help the end user to keep things simple, post from @tedsuo simply accelerated the discussion again, but if you pay a closer look at last comments api.withSpan was the last one. This is a helper that bounds things together from different namespaces, there can be more helpers that will mix things from other namespace. I don't think the helpers should sit in many different namespace because it will confuse user even more.

[Flarna]

anyone feel free to grap the change from my branch. I have no motivation to continue with this.