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

feat: extend configuration to support custom randomNumber func #555

Merged

Conversation

guanwenbogit
Copy link
Contributor

Which problem is this PR solving?

  • I want set tracer randomNumber when the config.New() called

Short description of the changes

  • Added the WithRandomNumber function and the randomNumber field for config.Options. Finally, apply the setting in the config.NewTracer() function

@guanwenbogit guanwenbogit changed the title feat: costume tracer randomNumber func on config.New() called feat: customize tracer randomNumber func on config.New() called Dec 18, 2020
@codecov
Copy link

codecov bot commented Dec 18, 2020

Codecov Report

Merging #555 (4e06150) into master (17fd3e8) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #555      +/-   ##
==========================================
+ Coverage   88.59%   88.61%   +0.01%     
==========================================
  Files          61       61              
  Lines        3307     3311       +4     
==========================================
+ Hits         2930     2934       +4     
  Misses        251      251              
  Partials      126      126              
Impacted Files Coverage Δ
config/config.go 95.33% <100.00%> (+0.06%) ⬆️
config/options.go 100.00% <100.00%> (ø)

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 17fd3e8...4e06150. Read the comment docs.

Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks fine, but let's add some tests to cover the new functionality.

@@ -147,6 +148,13 @@ func Extractor(format interface{}, extractor jaeger.Extractor) Option {
}
}

// WithRandonNunmber set the Tracer random number func
func WithRandonNunmber(f func() uint64) Option {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: WithRandomNumber

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Emmmmmmmm, my mistake.

@guanwenbogit guanwenbogit force-pushed the feature/with_random_number branch from d1cab53 to 85164cb Compare December 21, 2020 02:20
config/options.go Outdated Show resolved Hide resolved
@yurishkuro
Copy link
Member

Please make sure all commits are signed. Might be easier to squash them into one. See CONTRIBUTING.md.

@guanwenbogit guanwenbogit force-pushed the feature/with_random_number branch from 821b841 to 71e806a Compare December 21, 2020 12:15
}
randomNum := func() uint64 { return traceID }
tracer, closer, err := cfg.NewTracer(WithRandomNumber(randomNum))
span:=tracer.StartSpan("test-span")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs make fmt

spanCtx := span.Context().(jaeger.SpanContext)

assert.NoError(t, err)
assert.Equal(t, spanCtx.TraceID().Low ,traceID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: wrong argument order, expected value (traceID) goes first

Signed-off-by: guanwenbo <[email protected]>
@guanwenbogit guanwenbogit force-pushed the feature/with_random_number branch from 6078ed8 to 379fcac Compare December 23, 2020 10:48
Signed-off-by: guanwenbo <[email protected]>
@guanwenbogit guanwenbogit force-pushed the feature/with_random_number branch from 379fcac to 4e06150 Compare December 23, 2020 10:54
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@yurishkuro yurishkuro changed the title feat: customize tracer randomNumber func on config.New() called feat: extend configuration to support custom randomNumber func Dec 23, 2020
@yurishkuro yurishkuro merged commit fe3fa55 into jaegertracing:master Dec 23, 2020
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.

3 participants