Clean up ProbabilitySampler for 64 bit trace IDs#238
Clean up ProbabilitySampler for 64 bit trace IDs#238c24t merged 11 commits intoopen-telemetry:masterfrom
Conversation
c24t
requested review from
Oberon00,
a-feld,
carlosalberto,
lzchen,
reyang and
toumorokoshi
as code owners
This change because mypy isn't willing to believe the exponent is always positive. See python/typeshed#285.
c24t
changed the title
|
I reverted most of the changes in f2251ea now that we've decided to use the low bits for sampling decisions for compatibility with 64 bit trace IDs in w3c/trace-context#349. This is now mostly a cosmetic and testing change. |
c24t
added
the
needs reviewers
Codecov Report
@@ Coverage Diff @@
## master #238 +/- ##
=========================================
Coverage ? 85.32%
=========================================
Files ? 38
Lines ? 1956
Branches ? 231
=========================================
Hits ? 1669
Misses ? 221
Partials ? 66Continue to review full report at Codecov.
|
codeboten
left a comment
codeboten
left a comment
There was a problem hiding this comment.
Change looks good, just one question and a typo fix.
Co-Authored-By: alrex <alrex.boten@gmail.com>
mauriciovasquezbernal
left a comment
mauriciovasquezbernal
left a comment
There was a problem hiding this comment.
LGTM, just a couple of nits.
| 0xDEADBEF0, 0xDEADBEF1, trace_options=TO_SAMPLED | ||
| ), | ||
| 0x8000000000000001, | ||
| 0x8000000000000000, |
There was a problem hiding this comment.
Would 0x0 make it more clear?
There was a problem hiding this comment.
Same reason as above, but not a strong preference in either case.
Co-authored-by: alrex <alrex.boten@gmail.com>
* feat: add BatchSpanProcessor * fix: remove default value, already done Ctor * fix: append the MS unit * fix: move unrefTimer function in core * fix: use SetInterval instead of SetTimer * fix: rename TraceOptions -> TraceFlags * Update packages/opentelemetry-basic-tracer/src/export/BatchSpanProcessor.ts Co-Authored-By: Olivier Albertini <olivier.albertini@montreal.ca> * fix: remove _MS * fix: rename _lastSpanWrite -> _lastSpanFlush
4 participants
Follow up to #225.
This PR changes
ProbabilitySamplerso that it checks the left bits of the trace ID (following open-telemetry/opentelemetry-specification#331) instead of the right bits (like OpenCensus).Whether this is the right behavior depends on open-telemetry/opentelemetry-specification#331, and does break from the other clients.