feat: add OTEL_SAMPLING_PROBABILITY env var

[naseemkullah]

User can now control the trace sampling rate by means of env var.

JS implementation of open-telemetry/opentelemetry-specification#567

[naseemkullah]

Hello, I cannot seem to get OTEL_SAMPLING_RATE to work for the BasicTracerProvider test, any suggestions?

[vmarchaud]

by default _config is the DEFAULT_CONFIG so your fallback is never called here

[naseemkullah]

Thanks @vmarchaud I did this because of ts error:

Type 'Sampler | undefined' is not assignable to type 'Sampler'.
  Type 'undefined' is not assignable to type 'Sampler'.

But have worked around this with:

readonly sampler: api.Sampler | undefined;

[vmarchaud]

We should always have a sampler though so you approach was fine, however i would use ?? instead of ||

EDIt: The current approach is the correct way, but you shouldn't need to add | undefined to the _sampler

[vmarchaud]

This config is created when this file is required so well before your test tries to set it into the environment.
I don't think you should change the default config. I would have checked the existence of the env variable inside the BasicTracer and override the sampler there. Also you'll like the other PR to have a different implementation for the browser since process will not be available

[naseemkullah]

This config is created when this file is required so well before your test tries to set it into the environment.
I don't think you should change the default config. I would have checked the existence of the env variable inside the BasicTracer and override the sampler there.

Also tried this approach but the env var is not working as expected 🤔

  1) BasicTracerProvider
       .startSpan()
         should not sample a trace when OTEL_SAMPLING_RATE is 0:

      AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
+ expected - actual

- 1
+ 0

[vmarchaud]

I don't know then, maybe try to use the ConsoleLogger and increase the verbosity to debug ?

[naseemkullah]

Hmmm in the end I only had to make changes to Tracer.ts but not BasicTracerProvider,ts 🤷‍♂️

[codecov-io]

Codecov Report

Merging #975 into master will increase coverage by 1.46%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #975      +/-   ##
==========================================
+ Coverage   95.02%   96.49%   +1.46%     
==========================================
  Files         209      189      -20     
  Lines        8541     7757     -784     
  Branches      766      627     -139     
==========================================
- Hits         8116     7485     -631     
+ Misses        425      272     -153     

Impacted Files	Coverage Δ
packages/opentelemetry-tracing/src/Tracer.ts	100.00% <100.00%> (ø)
...telemetry-tracing/test/BasicTracerProvider.test.ts	100.00% <100.00%> (ø)
...ckages/opentelemetry-core/src/common/NoopLogger.ts	60.00% <0.00%> (-20.00%)	⬇️
...metry-core/src/trace/instrumentation/BasePlugin.ts	81.57% <0.00%> (-5.27%)	⬇️
...es/opentelemetry-exporter-collector/src/version.ts
...es/opentelemetry-exporter-collector/test/helper.ts
...entelemetry-plugin-xml-http-request/src/version.ts
packages/opentelemetry-web/src/utils.ts
...ry-context-zone-peer-dep/src/ZoneContextManager.ts
...y-exporter-collector/test/browser/index-webpack.ts
... and 14 more

[naseemkullah]

Not sure how 637f982 wound up in this branch. I tried rebasing from master.

[naseemkullah]

Anyone know why isn't CI being triggered?

[naseemkullah]

Closing and opening a new PR due to git complications.