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: Relax TransactionNameProvider. #1861

Merged
merged 4 commits into from
Jan 18, 2022

Conversation

maciejwalkowiak
Copy link
Contributor

📜 Description

Turn TransactionNameProvider into an interface enabling using SentryTracingFilter with other MVC frameworks like Grails or JAX-RS.

💡 Motivation and Context

💚 How did you test it?

Unit & Integration tests

📝 Checklist

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

For great majority of users it's not a breaking change. It is breaking only because the SentryTracingFilter secondary constructor takes now an interface instead of a class.

Turn TransactionNameProvider into an interface enabling using SentryTracingFilter with other MVC frameworks like Grails or JAX-RS.
@maciejwalkowiak maciejwalkowiak marked this pull request as ready for review January 4, 2022 05:53
@codecov-commenter
Copy link

codecov-commenter commented Jan 4, 2022

Codecov Report

Merging #1861 (b8240e8) into 6.x.x (fdbaed7) will decrease coverage by 0.00%.
The diff coverage is 91.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##              6.x.x    #1861      +/-   ##
============================================
- Coverage     80.69%   80.68%   -0.01%     
  Complexity     2897     2897              
============================================
  Files           214      214              
  Lines         10674    10682       +8     
  Branches       1399     1399              
============================================
+ Hits           8613     8619       +6     
- Misses         1565     1566       +1     
- Partials        496      497       +1     
Impacted Files Coverage Δ
...ring/tracing/SpringMvcTransactionNameProvider.java 66.66% <66.66%> (ø)
...io/sentry/spring/boot/SentryAutoConfiguration.java 98.38% <100.00%> (+0.05%) ⬆️
...java/io/sentry/spring/SentryExceptionResolver.java 100.00% <100.00%> (ø)
...main/java/io/sentry/spring/SentryHubRegistrar.java 93.18% <100.00%> (+0.15%) ⬆️
...ring/SentryRequestHttpServletRequestProcessor.java 100.00% <100.00%> (ø)
...main/java/io/sentry/spring/SentrySpringFilter.java 72.22% <100.00%> (+1.06%) ⬆️
.../io/sentry/spring/SentrySpringRequestListener.java 81.81% <100.00%> (+0.86%) ⬆️
.../io/sentry/spring/tracing/SentryTracingFilter.java 81.57% <100.00%> (ø)
...n/java/io/sentry/transport/ReusableCountLatch.java 88.23% <0.00%> (-5.89%) ⬇️

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 fdbaed7...b8240e8. Read the comment docs.

@marandaneto
Copy link
Contributor

@maciejwalkowiak please merge 6.x.x into your branch, I will be moving 6.0.0 forward.

@jsolas
Copy link

jsolas commented Apr 26, 2022

How could I use SentryTracingFilter in grails 2 ?

@maciejwalkowiak
Copy link
Contributor Author

Since we do not provide official support for Grails 2 (it's like 10 years old?) I can only give some hints:

  • include SentryTracingFilter in web.xml or any other place where you can declare servlet filters
  • assuming that it works similar to Grails 4, make sure that it runs after any Grails component that sets controller name on a HttpServletRequest attribute

If you are not able to use servlet filters, you will likely need to create custom Grails filter as explained in Grails 2.2. docs and you can use SentryTracingFilter as a reference, as the logic will be likely the same.

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.

4 participants