Move suppress tracing context key to SDK

[dyladan]

Originally we talked about putting this in an API extensions package, but because it is a feature of the SDK I think it should be a part of the SDK, not an external component. It can be made available to instrumentations by either an API extensions package later or by instrumentations depending on core. Because we use Symbol.for to get keys, there is no version issue with this.

API extensions package can be added as a follow-up

[codecov]

Codecov Report

Merging #2202 (878f0e8) into main (1745d87) will decrease coverage by 0.09%.
The diff coverage is 96.77%.

@@            Coverage Diff             @@
##             main    #2202      +/-   ##
==========================================
- Coverage   92.81%   92.71%   -0.10%     
==========================================
  Files         139      142       +3     
  Lines        5021     5120      +99     
  Branches     1031     1049      +18     
==========================================
+ Hits         4660     4747      +87     
- Misses        361      373      +12     

Impacted Files	Coverage Δ
...s/opentelemetry-core/src/trace/suppress-tracing.ts	87.50% <87.50%> (ø)
...e/src/baggage/propagation/HttpBaggagePropagator.ts	96.96% <100.00%> (+0.09%)	⬆️
...metry-core/src/trace/HttpTraceContextPropagator.ts	100.00% <100.00%> (ø)
...ges/opentelemetry-instrumentation-http/src/http.ts	95.63% <100.00%> (+0.01%)	⬆️
...entelemetry-propagator-b3/src/B3MultiPropagator.ts	100.00% <100.00%> (ø)
...es/opentelemetry-propagator-b3/src/B3Propagator.ts	100.00% <100.00%> (ø)
...ntelemetry-propagator-b3/src/B3SinglePropagator.ts	100.00% <100.00%> (ø)
...elemetry-propagator-jaeger/src/JaegerPropagator.ts	100.00% <100.00%> (ø)
packages/opentelemetry-tracing/src/Tracer.ts	100.00% <100.00%> (ø)
...telemetry-tracing/src/export/BatchSpanProcessor.ts	91.20% <100.00%> (ø)
... and 5 more

[vmarchaud]

lgtm, lint is failing though

[obecny]

I will not block it but I still think this is bad idea to add this to sdk. Adding this to sdk will force sooner or later to use sdk everywhere in each package. Now we have very good separation and dependency. With this one small change we will add extra 200k to the web too and to any package just to be able to use one small functionality. Not mentioning that any 3rd party sdk which until now was using only api and instrumentation and semantic conventions will have to include our sdk next to its own sdk. More kb, possible name / functionality collisions etc. etc. I would encourage to reconsider this now as it will be much harder later to remove it.

[dyladan]

Adding this to sdk will force sooner or later to use sdk everywhere in each package.

This only applies to instrumentations because the rest of the SDK already depends on core.

Of all of our instrumentations, only 1 has it so saying "everywhere in each package" will have to eventually depend on the SDK is a little hyperbolic.

Now we have very good separation and dependency. With this one small change we will add extra 200k to the web too and to any package just to be able to use one small functionality.

There are no web instrumentations which use suppress currently. The web sdk already depends on core. To me it seems there is no change for web.

Not mentioning that any 3rd party sdk which until now was using only api and instrumentation and semantic conventions will have to include our sdk next to its own sdk. More kb, possible name / functionality collisions etc. etc. I would encourage to reconsider this now as it will be much harder later to remove it.

Third party SDKs can make their own decisions but there is no requirement for them to use this feature or functionality. Remember that the exporters, processors, and propagators are considered to be written for our SDK. If a third party SDK decides to use them then great, but there is no requirement there.

[dyladan]

Broken tests appear unrelated to this change. main is also broken locally for me.

[dyladan]

I think this should be merged even though the tests are broken because the test failures are unrelated (they happen on other PRs and on main too) and this is blocking the upgrade to use api 0.19

@open-telemetry/javascript-maintainers WDYT

[obecny]

I think this should be merged even though the tests are broken because the test failures are unrelated (they happen on other PRs and on main too) and this is blocking the upgrade to use api 0.19

@open-telemetry/javascript-maintainers WDYT

the linting is failing too, I'm checking will let you know what I find out

[dyladan]

Sorry for force push. Had to remove the commits which provided all the linting "fixes" for the bad prettier linting rules.

[dyladan]

@Flarna yeah its related to the rebase. working on it now

[dyladan]

Should be OK now