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

Sentry scope is lost with async Spring controllers #1226

Closed
1 of 27 tasks
dparrella opened this issue Feb 4, 2021 · 6 comments · Fixed by #1652
Closed
1 of 27 tasks

Sentry scope is lost with async Spring controllers #1226

dparrella opened this issue Feb 4, 2021 · 6 comments · Fixed by #1652
Labels
enhancement New feature or request Spring Spring

Comments

@dparrella
Copy link

Platform:

  • Android -> If yes, which Device API (and compileSdkVersion/targetSdkVersion/Build tools) version?
  • Java -> If yes, which Java (and sourceCompatibility/targetCompatibility) version?
  • Kotlin -> If yes, which Kotlin (and jvmTarget) version?
  • NDK -> If yes, which NDK/CMake version?
  • React-Native -> If yes, which version?
  • Timber -> If yes, which version?
  • Log4j2 -> If yes, which version?
  • Logback -> If yes, which version?
  • Spring -> 5.3.3

IDE:

  • Android Studio -> If yes, which version?
  • IntelliJ -> If yes, which version?
  • Other -> If yes, which one?

Build system:

  • Gradle -> If yes, which version?
  • Buck -> If yes, which version?
  • Bazel -> If yes, which version?
  • Maven -> If yes, which version?
  • Other -> If yes, which one?

Android Gradle Plugin:

  • Yes -> If yes, which version?
  • No

Sentry Android Gradle Plugin:

  • Yes -> If yes, which version?
  • No

Proguard/R8:

  • Enabled
  • Disabled

Platform installed with:

  • JCenter
  • Bintray
  • Maven Central
  • Manually

The version of the SDK:
4.0.0


I have the following issue:

We have noticed that issues generated from async requests our spring controllers lose context. This means we don't see things like the request information. I think it also breaks tracing.

I tried using a TaskDecorator (we have one for copying MDC context) but there is no way to pass in a "copy" of the current scope to the Runnable we return. I think ideally we'd have a Sentry.setScope() or a merge() method on Scope.

Example TaskDecorator:

public Runnable decorate(final Runnable runnable) {
    final AtomicReference<Scope> currentScope = new AtomicReference<>();
    Sentry.configureScope(scope -> currentScope.set(scope));
    return () -> {
        // Copy currentScope in here somehow
        runnable.run();
    };
}

Example Async controller method:

    @GetMapping("/v1/hello")
    @NoPreAuthorize
    public Callable<String> sayHello() {
        return () -> {
            log.error("Testing logback integration");
            return this.helloService.sayHello();
        };
    }

Steps to reproduce:

  • See code samples.

Actual result:

  • Issue is created without any request context.

Expected result:

  • We should have something that allows us to "copy" over the current scope into another thread.
@marandaneto
Copy link
Contributor

I guess this is a dup of #1107 could you confirm @dparrella ?

@dparrella
Copy link
Author

It's similar but it's not exactly the same. That issue refers to a StreamingResponseBody while this is about Spring MVC's Async request handling. The ask is the same: we need a way to copy Scope from one thread to another. The current Scope object's methods do not support this.

@marandaneto marandaneto added enhancement New feature or request Spring Spring and removed waiting for feedback labels Feb 5, 2021
@marandaneto
Copy link
Contributor

agreed, ideally, we'd make something generic enough to tackle both problems.

@dparrella
Copy link
Author

Any updates on this issue? This is a major problem for us.

@maciejwalkowiak
Copy link
Contributor

For the coroutines implementation we added methods: Sentry#getCurrentHub an Sentry#setCurrentHub - both are marked as internal as they were initially not meant to be used by end users. I am not 100% sure but perhaps you can use them to solve this problem.

@bruno-garcia
Copy link
Member

If there's no 'framework' API we can hook to do the calls to getCurrentHub and setCurrentHub ourselves I'm afraid there's just not way the SDK can propagate it automagically.

But as @maciejwalkowiak suggested you could use that API at the right places to propagate state. Would that work for you @dparrella?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Spring Spring
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants