POTEL 30 - span.startChild now uses .makeCurrent() by default

[adinauer]

📜 Description

When starting a child span of a OtelSpanWrapper span (i.e. use Sentry API and OpenTelemetry under the hood), we now call .makeCurrent() on the child span by default so it's made the current span.

💡 Motivation and Context

Without this, there's a difference in behaviour when using OpenTelemetry. Making a span current was previously part of the transaction which can or cannot be the current transaction on the stack.

We could follow up on this by exposing a flag on SpanOptions that allows customers to control this. We already have bindToScope for TransactionOptions which defaults to false. For backend IMO it makes much more sense to bind to scope by default. This option can then also be used for Sentry.startSpan and Sentry.startInactiveSpan.

💚 How did you test it?

📝 Checklist

• I reviewed the submitted code.
• I added tests to verify the changes.
• No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
• I updated the docs if needed.
• Review from the native team if needed.
• No breaking change or entry added to the changelog.
• No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

[github-actions]

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against bcae360

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	399.71 ms	467.16 ms	67.45 ms
Size	1.70 MiB	2.29 MiB	599.77 KiB

Baseline results on branch: 8.x.x

Startup times

Revision	Plain	With Sentry	Diff
a59fca2	560.08 ms	658.80 ms	98.71 ms

App size

Revision	Plain	With Sentry	Diff
a59fca2	1.70 MiB	2.29 MiB	599.77 KiB

Previous results on branch: feat/potel-30-child-span-make-current-by-default

Startup times

Revision	Plain	With Sentry	Diff
be5a4cc	391.08 ms	457.08 ms	66.00 ms

App size

Revision	Plain	With Sentry	Diff
be5a4cc	1.70 MiB	2.29 MiB	599.77 KiB

[stefanosiano]

this should also use makeCurrent(), even if there is no practical change, right?

[adinauer]

Yeah NoOpSpan.makeCurrent does nothing and returns a NoOpScopesLifecycleToken atm.

[adinauer]

I just checked, there's more places where we would have to add this .makeCurrent for noop spans. I'd say we only add it when we actually need it. That's if we ever change NoOpSpan.makeCurrent to modify Context.