Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Add breadcrumb in Spring RestTemplate integration #1481

Merged
merged 2 commits into from
May 19, 2021

Conversation

maciejwalkowiak
Copy link
Contributor

📜 Description

Feat: Add breadcrumb in Spring RestTemplate integration

💡 Motivation and Context

In the process of alignment OkHttp integration with Spring RestTemplate integration I noticed that we do not add breadcrumb for http request going through RestTemplate. With this change, breadcrumb is added no matter if there is an active ongoing transaction.

💚 How did you test it?

Unit tests.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

@maciejwalkowiak maciejwalkowiak marked this pull request as ready for review May 19, 2021 08:03
final @Nullable Integer responseStatusCode) {
final Breadcrumb breadcrumb =
Breadcrumb.http(request.getURI().toString(), request.getMethodValue(), responseStatusCode);
breadcrumb.setData("requestBodySize", body.length);
Copy link
Contributor

Choose a reason for hiding this comment

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

can we read the response and if available, also adding responseBodySize?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

response is an input stream here, to get the size we would need to read it.

@codecov-commenter
Copy link

codecov-commenter commented May 19, 2021

Codecov Report

Merging #1481 (88d48f9) into main (657706a) will decrease coverage by 0.09%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1481      +/-   ##
============================================
- Coverage     75.75%   75.66%   -0.10%     
  Complexity     1926     1926              
============================================
  Files           190      190              
  Lines          6580     6588       +8     
  Branches        665      665              
============================================
  Hits           4985     4985              
- Misses         1270     1278       +8     
  Partials        325      325              
Impacted Files Coverage Δ Complexity Δ
...racing/SentrySpanClientHttpRequestInterceptor.java 0.00% <0.00%> (ø) 0.00 <0.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 657706a...88d48f9. Read the comment docs.

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

LGTM

@maciejwalkowiak maciejwalkowiak merged commit c68d7be into main May 19, 2021
@maciejwalkowiak maciejwalkowiak deleted the resttemplate-breadcrumb branch May 19, 2021 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants