Skip to content

Conversation

@GoncaloGarcia
Copy link
Contributor

@GoncaloGarcia GoncaloGarcia commented Nov 26, 2019

With sampling we can reduce the tracing overhead by limiting the
percentage of requests that are traced.

Since Jaeger's sampling functionality does not ignore the
instrumentation code we had to implement our own sampling.

This is done by generating a thread local random every time we call
addToTrace and if this value is below the sampling threshold a baggage
item ("sampled" : "True") is added to the span. From then on if the
active span's baggage does not contain that item, the instrumentation
code will be ignored and when necessary Noop versions of Span, Scope and
SpanContext are returned.

With sampling we can reduce the tracing overhead by limiting the
percentage of requests that are traced.

Since Jaeger's sampling functionality does not ignore the
instrumentation code we had to implement our own sampling.

This is done by generating a thread local random every time we call
addToTrace and if this value is below the sampling threshold a baggage
item ("sampled" : "True") is added to the span. From then on if the
active span's baggage does not contain that item, the instrumentation
code will be ignored and when necessary Noop versions of Span, Scope and
SpanContext are returned.
@codecov
Copy link

codecov bot commented Nov 26, 2019

Codecov Report

Merging #16 into master will decrease coverage by 11.09%.
The diff coverage is 41.91%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #16      +/-   ##
============================================
- Coverage     71.27%   60.17%   -11.1%     
- Complexity       90       92       +2     
============================================
  Files             7       16       +9     
  Lines           369      452      +83     
  Branches         23       25       +2     
============================================
+ Hits            263      272       +9     
- Misses           85      150      +65     
- Partials         21       30       +9
Impacted Files Coverage Δ Complexity Δ
...acing/engine/opentracing/noop/NoopSpanBuilder.java 0% <0%> (ø) 0 <0> (?)
...g/engine/opentracing/noop/NoopSpanContextImpl.java 0% <0%> (ø) 0 <0> (?)
...mons/tracing/engine/opentracing/noop/NoopSpan.java 0% <0%> (ø) 0 <0> (?)
...racing/engine/opentracing/noop/NoopTracerImpl.java 0% <0%> (ø) 0 <0> (?)
...g/engine/opentracing/noop/NoopSpanBuilderImpl.java 0% <0%> (ø) 0 <0> (?)
.../engine/opentracing/noop/NoopScopeManagerImpl.java 0% <0%> (ø) 0 <0> (?)
.../tracing/engine/opentracing/noop/NoopSpanImpl.java 0% <0%> (ø) 0 <0> (?)
...cing/engine/opentracing/noop/NoopScopeManager.java 0% <0%> (ø) 0 <0> (?)
...ing/engine/opentracing/noop/NoopTracerFactory.java 0% <0%> (ø) 0 <0> (?)
...racing/engine/configuration/BaseConfiguration.java 100% <100%> (ø) 4 <4> (?)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0bc938...6a1c48e. Read the comment docs.

@GoncaloGarcia
Copy link
Contributor Author

There might be a small issue with this pull request as it relies on the OpenTracing Noop implementation which isn't public (the API is, but the implementation classes aren't) so I had to download the code from their Github and include it in ours (making the implementation classes public in the process). Not sure how licensing works here but they use Apache 2.0 as far as I know.

@codecov-io
Copy link

Codecov Report

Merging #16 into master will decrease coverage by 11.09%.
The diff coverage is 41.91%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #16      +/-   ##
============================================
- Coverage     71.27%   60.17%   -11.1%     
- Complexity       90       92       +2     
============================================
  Files             7       16       +9     
  Lines           369      452      +83     
  Branches         23       25       +2     
============================================
+ Hits            263      272       +9     
- Misses           85      150      +65     
- Partials         21       30       +9
Impacted Files Coverage Δ Complexity Δ
...acing/engine/opentracing/noop/NoopSpanBuilder.java 0% <0%> (ø) 0 <0> (?)
...g/engine/opentracing/noop/NoopSpanContextImpl.java 0% <0%> (ø) 0 <0> (?)
...mons/tracing/engine/opentracing/noop/NoopSpan.java 0% <0%> (ø) 0 <0> (?)
...racing/engine/opentracing/noop/NoopTracerImpl.java 0% <0%> (ø) 0 <0> (?)
...g/engine/opentracing/noop/NoopSpanBuilderImpl.java 0% <0%> (ø) 0 <0> (?)
.../engine/opentracing/noop/NoopScopeManagerImpl.java 0% <0%> (ø) 0 <0> (?)
.../tracing/engine/opentracing/noop/NoopSpanImpl.java 0% <0%> (ø) 0 <0> (?)
...cing/engine/opentracing/noop/NoopScopeManager.java 0% <0%> (ø) 0 <0> (?)
...ing/engine/opentracing/noop/NoopTracerFactory.java 0% <0%> (ø) 0 <0> (?)
...racing/engine/configuration/BaseConfiguration.java 100% <100%> (ø) 4 <4> (?)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d79839...6a1c48e. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants