Skip to content

WIP: customizer all the things#1436

Closed
codefromthecrypt wants to merge 1 commit intomasterfrom
tracing-customizer
Closed

WIP: customizer all the things#1436
codefromthecrypt wants to merge 1 commit intomasterfrom
tracing-customizer

Conversation

@codefromthecrypt
Copy link
Contributor

This adds hooks for new customizers that will be in Brave 5.7. As I'll
be offline for a day or two, if anyone wants to play with this they can.

We could just weave-in and document this, or we could retrofit some of
our existing stuff to configure itself via customizers instead (ex MDC
integration).

cc @devinsba

@codecov
Copy link

codecov bot commented Aug 31, 2019

Codecov Report

Merging #1436 into master will decrease coverage by 0.04%.
The diff coverage is 36.84%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1436      +/-   ##
============================================
- Coverage     57.61%   57.57%   -0.05%     
- Complexity      794      795       +1     
============================================
  Files           150      150              
  Lines          4287     4304      +17     
  Branches        460      464       +4     
============================================
+ Hits           2470     2478       +8     
- Misses         1606     1612       +6     
- Partials        211      214       +3
Impacted Files Coverage Δ Complexity Δ
...loud/sleuth/autoconfig/TraceAutoConfiguration.java 69.87% <25%> (-7.59%) 21 <0> (ø)
...uth/instrument/web/TraceHttpAutoConfiguration.java 73.8% <57.14%> (-4.57%) 8 <0> (ø)
...trument/reactor/TraceReactorAutoConfiguration.java 92% <0%> (+4%) 1% <0%> (ø) ⬇️
...uth/instrument/async/TraceableExecutorService.java 76.74% <0%> (+4.65%) 23% <0%> (+1%) ⬆️

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 6ce2df4...5745a66. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 31, 2019

Codecov Report

Merging #1436 into master will decrease coverage by 0.11%.
The diff coverage is 36.84%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1436      +/-   ##
============================================
- Coverage     57.23%   57.12%   -0.12%     
- Complexity      777      779       +2     
============================================
  Files           148      148              
  Lines          4242     4259      +17     
  Branches        457      461       +4     
============================================
+ Hits           2428     2433       +5     
- Misses         1603     1611       +8     
- Partials        211      215       +4
Impacted Files Coverage Δ Complexity Δ
...loud/sleuth/autoconfig/TraceAutoConfiguration.java 69.87% <25%> (-7.59%) 21 <0> (ø)
...uth/instrument/web/TraceHttpAutoConfiguration.java 73.8% <57.14%> (-4.57%) 8 <0> (ø)

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 0cf9003...17fbff1. Read the comment docs.

@marcingrzejszczak
Copy link
Contributor

Lgtm

I'd refactor what we have to use the customizers I guess

@connectwithnara
Copy link

@marcingrzejszczak Netflix needs this. Could you include it in 2.1.3. Thanks.

@adriancole fyi

@shakuzen
Copy link
Contributor

shakuzen commented Sep 6, 2019

Brave 5.7 is being released now.

@codefromthecrypt
Copy link
Contributor Author

this is now on a stable release. needs tests and docs.. not sure if you have time for it @marcingrzejszczak but I'm offline until monday. hoping to get this in (also 2.1) to unlock the secondary sampling thing nara's hoping to roll out.

for (FinishedSpanHandler finishedSpanHandlerFactory : this.finishedSpanHandlers) {
builder.addFinishedSpanHandler(finishedSpanHandlerFactory);
}
for (TracingCustomizer customizer : this.tracingCustomizers) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ps without tests this is subtle. it is important these apply last as they can rejig the reporter config, something needed for secondary sampling to work. ex we want to flip default reporter to noop but reuse the normal reporter as a finished span handler. this stuff works when customizes hit last before build

@codefromthecrypt
Copy link
Contributor Author

codefromthecrypt commented Sep 8, 2019 via email

@codefromthecrypt codefromthecrypt deleted the tracing-customizer branch September 8, 2019 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants