Remove SPI-based RequestContextStorageProvider lookup
#4211
Milestone
Comments
Merged
minwoox
pushed a commit
that referenced
this issue
Sep 5, 2022
…4232) Motivation: - Context leaks are hard to find because an exception does not tell where/which context is pushed without poping. By using TraceAbleRequestContextStorage, it helps to report the source of context leaks. - Details as mentioned in #4100 By the way, Thanks to @anuraaga for giving a reference to read on [opentelemetry](https://github.com/open-telemetry/opentelemetry-java). Modifications: - Add `TraceAbleRequestContextStorage` that stores `RequestContext` stack trace and reports to the user where it happens. - Add `requestContextLeakDetectionSampler` flag that users can use for enable leak detection. Users can enable it by either system property or SPI flag provider. Result: - Closes #4100 - `TraceAbleRequestContextStorage` is added, so users can use it to report where context leaks happen. How to enable: 1) By system property `-Dcom.linecorp.armeria.requestContextLeakDetectionSampler=<sampler-spec>` 2) By providing FlagsProvider SPI ```java public final class EnableLeakDetectionFlagsProvider implements FlagsProvider { @OverRide public Sampler<? super RequestContext> requestContextLeakDetectionSampler() { return Sampler.always(); } ... } ``` 3) By providing RequestContextStorageProvider SPI (not recommend since RequestContextStorageProvider SPI'll be remove as mentioned in ##4211 ) ```java public final class CustomRequestContextStorageProvider implements RequestContextStorageProvider { @OverRide public RequestContextStorage newStorage() { return new TraceAbleRequestContextStorage(delegate); } } ``` Use case: Users problematic code ```java executor.execute(() -> { SafeCloseable leaked = fooCtx.push(); //This causes Request context leaks! ... }); executor.execute(() -> { try (SafeCloseable ignored = barCtx.push()) { //Exception happen here ... } }); ``` The above code will produce an error as below. Therefore, users can check the stack trace that which line causes context leaks. ``` java.lang.IllegalStateException: Trying to call object wrapped with context [%New RequestContext%], but context is currently set to TraceableServiceRequestContext[%Previous RequestContext%] com.linecorp.armeria.internal.common.LeakTracingRequestContextStorage$PendingRequestContextStackTrace: At thread [armeria-testing-eventloop-nio-1-1] previous RequestContext is pushed at the following stacktrace at com.linecorp.armeria.internal.common.LeakTracingRequestContextStorage$TraceableServiceRequestContext.<init>(LeakTracingRequestContextStorage.java:111) at com.linecorp.armeria.internal.common.LeakTracingRequestContextStorage$TraceableServiceRequestContext.<init>(LeakTracingRequestContextStorage.java:105) at com.linecorp.armeria.internal.common.LeakTracingRequestContextStorage.warpRequestContext(LeakTracingRequestContextStorage.java:82) at com.linecorp.armeria.internal.common.LeakTracingRequestContextStorage.push(LeakTracingRequestContextStorage.java:62) at com.linecorp.armeria.internal.common.RequestContextUtil.getAndSet(RequestContextUtil.java:149) at com.linecorp.armeria.server.ServiceRequestContext.push(ServiceRequestContext.java:221) at com.linecorp.armeria.internal.common.TraceRequestContextLeakTest.lambda$singleThreadContextLeak$2(TraceRequestContextLeakTest.java:101) <- This is the line where leaked RequestContext is push ... . This means the callback was called from unexpected thread or forgetting to close previous context. at com.linecorp.armeria.internal.common.RequestContextUtil.newIllegalContextPushingException(RequestContextUtil.java:100) at com.linecorp.armeria.server.ServiceRequestContext.push(ServiceRequestContext.java:237) at com.linecorp.armeria.internal.common.TraceRequestContextLeakTest.lambda$singleThreadContextLeak$3(TraceRequestContextLeakTest.java:107) ... ```
heowc
pushed a commit
to heowc/armeria
that referenced
this issue
Sep 24, 2022
…ine#4232) Motivation: - Context leaks are hard to find because an exception does not tell where/which context is pushed without poping. By using TraceAbleRequestContextStorage, it helps to report the source of context leaks. - Details as mentioned in line#4100 By the way, Thanks to @anuraaga for giving a reference to read on [opentelemetry](https://github.com/open-telemetry/opentelemetry-java). Modifications: - Add `TraceAbleRequestContextStorage` that stores `RequestContext` stack trace and reports to the user where it happens. - Add `requestContextLeakDetectionSampler` flag that users can use for enable leak detection. Users can enable it by either system property or SPI flag provider. Result: - Closes line#4100 - `TraceAbleRequestContextStorage` is added, so users can use it to report where context leaks happen. How to enable: 1) By system property `-Dcom.linecorp.armeria.requestContextLeakDetectionSampler=<sampler-spec>` 2) By providing FlagsProvider SPI ```java public final class EnableLeakDetectionFlagsProvider implements FlagsProvider { @OverRide public Sampler<? super RequestContext> requestContextLeakDetectionSampler() { return Sampler.always(); } ... } ``` 3) By providing RequestContextStorageProvider SPI (not recommend since RequestContextStorageProvider SPI'll be remove as mentioned in #line#4211 ) ```java public final class CustomRequestContextStorageProvider implements RequestContextStorageProvider { @OverRide public RequestContextStorage newStorage() { return new TraceAbleRequestContextStorage(delegate); } } ``` Use case: Users problematic code ```java executor.execute(() -> { SafeCloseable leaked = fooCtx.push(); //This causes Request context leaks! ... }); executor.execute(() -> { try (SafeCloseable ignored = barCtx.push()) { //Exception happen here ... } }); ``` The above code will produce an error as below. Therefore, users can check the stack trace that which line causes context leaks. ``` java.lang.IllegalStateException: Trying to call object wrapped with context [%New RequestContext%], but context is currently set to TraceableServiceRequestContext[%Previous RequestContext%] com.linecorp.armeria.internal.common.LeakTracingRequestContextStorage$PendingRequestContextStackTrace: At thread [armeria-testing-eventloop-nio-1-1] previous RequestContext is pushed at the following stacktrace at com.linecorp.armeria.internal.common.LeakTracingRequestContextStorage$TraceableServiceRequestContext.<init>(LeakTracingRequestContextStorage.java:111) at com.linecorp.armeria.internal.common.LeakTracingRequestContextStorage$TraceableServiceRequestContext.<init>(LeakTracingRequestContextStorage.java:105) at com.linecorp.armeria.internal.common.LeakTracingRequestContextStorage.warpRequestContext(LeakTracingRequestContextStorage.java:82) at com.linecorp.armeria.internal.common.LeakTracingRequestContextStorage.push(LeakTracingRequestContextStorage.java:62) at com.linecorp.armeria.internal.common.RequestContextUtil.getAndSet(RequestContextUtil.java:149) at com.linecorp.armeria.server.ServiceRequestContext.push(ServiceRequestContext.java:221) at com.linecorp.armeria.internal.common.TraceRequestContextLeakTest.lambda$singleThreadContextLeak$2(TraceRequestContextLeakTest.java:101) <- This is the line where leaked RequestContext is push ... . This means the callback was called from unexpected thread or forgetting to close previous context. at com.linecorp.armeria.internal.common.RequestContextUtil.newIllegalContextPushingException(RequestContextUtil.java:100) at com.linecorp.armeria.server.ServiceRequestContext.push(ServiceRequestContext.java:237) at com.linecorp.armeria.internal.common.TraceRequestContextLeakTest.lambda$singleThreadContextLeak$3(TraceRequestContextLeakTest.java:107) ... ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Armeria 1.17 will have a new flags SPI in #4151 that allows a user to override the default
RequestContextStorageProviderby writing aFlagsProvider. There's no need to confuse our users by having two SPI loaders.The text was updated successfully, but these errors were encountered: