Respect sampled bit in probability sampler

[dyladan]

Fixes #787

[dyladan]

/cc @Flarna

[Flarna]

I think we should respect the trace flag not only in the true case.

Maybe move this check to the tracer and call the sampler only for root spans?

[dyladan]

I think we should respect the trace flag not only in the true case.

Maybe move this check to the tracer and call the sampler only for root spans?

I don't want to move the logic to the tracer because you could theoretically want to implement a sampler that does not respect trace flags (perhaps you want to use some tracestate entry instead). I revised the logic, PTAL.

[Flarna]

Looks good. I agree that keeping it out of tracer is the better choice.

The checks in processors seem to be redundant as unsampled spans are not forwarded to processors anyway.

[dyladan]

@Flarna I removed the isSampled check from the span processors. I looked through the spec and don't see anywhere it says the span processor should be checking. I verified that you are correct, unsampled spans are indeed not sent to the span processor. I think whoever implemented the span processors was probably just being safe.

/cc @mayurkale22 WDYT about this decision?

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@fc287d3). Click here to learn what that means.
The diff coverage is 75%.

@@            Coverage Diff            @@
##             master     #798   +/-   ##
=========================================
  Coverage          ?   92.74%           
=========================================
  Files             ?      242           
  Lines             ?    10801           
  Branches          ?     1060           
=========================================
  Hits              ?    10017           
  Misses            ?      784           
  Partials          ?        0

Impacted Files	Coverage Δ
packages/opentelemetry-api/src/metrics/Metric.ts	100% <ø> (ø)
packages/opentelemetry-metrics/src/Metric.ts	95.12% <ø> (ø)
...y-api/test/noop-implementations/noop-meter.test.ts	100% <100%> (ø)
...metry-core/src/trace/sampler/ProbabilitySampler.ts	94.11% <66.66%> (ø)

[mayurkale22]

Overall lgtm, added a few comments

[obecny]

lgtm except one thing with undefined checking,