Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Allow setting of PoolSpans from Config object #322

Merged
merged 1 commit into from
Mar 23, 2019

Conversation

mdubbyap
Copy link
Contributor

Which problem is this PR solving?

  • lack of support for enabling pooling of spans from config object

Short description of the changes

  • add Config setter for PoolSpans and pipe it through to Tracer construction

@codecov
Copy link

codecov bot commented Jul 11, 2018

Codecov Report

Merging #322 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #322      +/-   ##
==========================================
+ Coverage   87.15%   87.16%   +0.01%     
==========================================
  Files          54       54              
  Lines        2965     2969       +4     
==========================================
+ Hits         2584     2588       +4     
  Misses        269      269              
  Partials      112      112
Impacted Files Coverage Δ
config/options.go 100% <100%> (ø) ⬆️
config/config.go 95.03% <100%> (+0.03%) ⬆️

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 f7e0d47...8ed34bf. Read the comment docs.

@yurishkuro
Copy link
Member

Have you actually tried using the pooling? I don't think anyone has really used / tested it.

@mdubbyap
Copy link
Contributor Author

It appears to work fine. Thanks for the heads up. I'll do a little more testing. Perhaps i'll add a metric for hit rate or something similar.

@yurishkuro yurishkuro merged commit 32e1d79 into jaegertracing:master Mar 23, 2019
@yurishkuro
Copy link
Member

Thanks, @mdubbyap !

FYI, see #381

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants