Skip to content

Do not instrument CGLIB proxies#1983

Merged
bsideup merged 3 commits into
masterfrom
do_not_instrument_proxies
Dec 9, 2019
Merged

Do not instrument CGLIB proxies#1983
bsideup merged 3 commits into
masterfrom
do_not_instrument_proxies

Conversation

@bsideup
Copy link
Copy Markdown
Contributor

@bsideup bsideup commented Dec 5, 2019

Since they are generated, they barely have any useful info,
and the debug info is usually missing anyways.

Also see spring-projects/spring-hateoas#1144

Since they are generated, they barely have any useful info,
and the debug info is usually missing anyways.

Also see spring-projects/spring-hateoas#1144
@bsideup bsideup added type/enhancement A general enhancement area/devexp This belongs to the development experience theme labels Dec 5, 2019
@bsideup bsideup added this to the 3.3.2.RELEASE milestone Dec 5, 2019
@bsideup
Copy link
Copy Markdown
Contributor Author

bsideup commented Dec 5, 2019

/cc @odrotbohm


Mono<Void> doSomething() {
Mono<Void> myMono = new MyMono();
if (myMono == null) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is that on purpose? shouldn't it be if (myMono != null) to be consistent?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is (see the comment)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the comment doesn't help me understand why there's a NPE triggered here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a dead code that will never get executed

But, since the operators are immutable, I removed the condition and it does not affect the test

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah I know it won't get executed, but since the npe is blatant it becomes a distraction and kind of forces the reader to wonder if the NPE is relevant in the test or not.

But, since the operators are immutable, I removed the condition and it does not affect the test

👍

@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 5, 2019

Codecov Report

Merging #1983 into master will increase coverage by 0.36%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1983      +/-   ##
============================================
+ Coverage     81.73%   82.09%   +0.36%     
- Complexity     4012     4330     +318     
============================================
  Files           375      375              
  Lines         30915    31671     +756     
  Branches       5808     5930     +122     
============================================
+ Hits          25267    26001     +734     
- Misses         4052     4064      +12     
- Partials       1596     1606      +10
Impacted Files Coverage Δ Complexity Δ
...ain/java/reactor/core/scheduler/SchedulerTask.java 73.21% <0%> (-3.58%) 16% <0%> (-1%)
...va/reactor/core/publisher/ParallelMergeReduce.java 69.67% <0%> (-1.64%) 3% <0%> (ø)
...r-core/src/main/java/reactor/core/Disposables.java 88.95% <0%> (-1.23%) 21% <0%> (ø)
.../main/java/reactor/core/publisher/MonoCollect.java 82.71% <0%> (-1.22%) 4% <0%> (+2%)
...va/reactor/core/publisher/MonoStreamCollector.java 80.64% <0%> (-1.18%) 4% <0%> (+2%)
.../main/java/reactor/core/publisher/FluxFlatMap.java 95.14% <0%> (+0.18%) 19% <0%> (ø) ⬇️
...ore/src/main/java/reactor/core/publisher/Flux.java 97.78% <0%> (+0.19%) 840% <0%> (+314%) ⬆️
...c/main/java/reactor/core/publisher/FluxReplay.java 84.29% <0%> (+0.3%) 28% <0%> (+1%) ⬆️
...in/java/reactor/core/publisher/FluxWindowWhen.java 86.05% <0%> (+0.48%) 2% <0%> (ø) ⬇️
...eactor/core/publisher/ParallelMergeSequential.java 85.49% <0%> (+0.51%) 7% <0%> (ø) ⬇️
... and 3 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 f614b1a...97f8683. Read the comment docs.

@bsideup bsideup merged commit 59aa6a4 into master Dec 9, 2019
@bsideup bsideup deleted the do_not_instrument_proxies branch December 9, 2019 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/devexp This belongs to the development experience theme type/enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants