Skip to content

Conversation

@bedla
Copy link
Contributor

@bedla bedla commented Jun 21, 2020

Hi,
I found that when I have request parameter that is optional (null when controller method invoked) with combination with @SpanTag, tag is handled incorrectly.
I have created PR that solves it. Please look at tests to see usecases (see NullParameterController).
What do you think?
Thx,
Ivos

@bedla bedla force-pushed the null-spantag-parameter branch from fa9dac9 to 62225e6 Compare June 21, 2020 18:56
@codecov
Copy link

codecov bot commented Jun 21, 2020

Codecov Report

Merging #1669 into master will increase coverage by 0.18%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1669      +/-   ##
============================================
+ Coverage     59.21%   59.40%   +0.18%     
- Complexity      768      771       +3     
============================================
  Files           134      134              
  Lines          4146     4148       +2     
  Branches        489      491       +2     
============================================
+ Hits           2455     2464       +9     
+ Misses         1466     1458       -8     
- Partials        225      226       +1     
Impacted Files Coverage Δ Complexity Δ
...ud/sleuth/annotation/SpanTagAnnotationHandler.java 86.76% <100.00%> (+3.43%) 26.00 <5.00> (+2.00)
...cloud/sleuth/instrument/reactor/ReactorSleuth.java 57.77% <0.00%> (+11.11%) 9.00% <0.00%> (+1.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 0b9ebf8...62225e6. Read the comment docs.

@marcingrzejszczak
Copy link
Contributor

marcingrzejszczak commented Jun 22, 2020

Lgtm can you apply this against 2.2.x branch?

@marcingrzejszczak
Copy link
Contributor

Hey @bedla , could you please apply this PR against 2.2.x branch?

@bedla
Copy link
Contributor Author

bedla commented Jun 30, 2020 via email

@marcingrzejszczak marcingrzejszczak merged commit ba83918 into spring-cloud:master Jun 30, 2020
@marcingrzejszczak
Copy link
Contributor

Ah I didn't know you're on holidays. I've merged your code to master and backported it to 2.2.x branch. Great work and enjoy your holidays :)

@bedla
Copy link
Contributor Author

bedla commented Jul 7, 2020

i am back :) thx for merge and sorry for inconvenience.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants