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

ObservationAwareSpanThreadLocalAccessor does not remove SpanAction #328

Closed
stalinone opened this issue Aug 11, 2023 · 2 comments
Closed
Labels
bug A general bug
Milestone

Comments

@stalinone
Copy link

Hi, I'm using Spring Boot 3.0.9 (w/ Micrometer Tracing 1.0.8) and configured to allow trace continued when @Async method or taskExecutor#execute is called.

I noticed ObservationAwareSpanThreadLocalAccessor#spanActions map is get bigger and bigger throughout time. It seems to me that SpanAction is still persisted in the map even if the Thread is already finished.

My configuration:

@Configuration
public class Config {

    @Autowired
    public Config(Tracer tracer) {
        ContextRegistry.getInstance()
                       .registerThreadLocalAccessor(new ObservationAwareSpanThreadLocalAccessor(tracer));
    }
    

    @Bean
    public ThreadPoolTaskExecutor someExecutor() {
        final ThreadPoolTaskExecutor taskExecutor =
                new TaskExecutorBuilder()/*...*/
                         .taskDecorator(runnable -> ContextSnapshot.captureAll(new Object[0]).wrap(runnable))
                         .build();
        taskExecutor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy());
        return taskExecutor;
    }
}

I'd like to know if there is additional configuration needs to be set, or is this a memory leak?

@stalinone
Copy link
Author

stalinone commented Aug 11, 2023

Also, when I checking the issue that leads to adding spanActions field, I found the below configuration could achieve trace continuity without using ObservationAwareSpanThreadLocalAccessor (at least it seems):

@Configuration
public class Config {

    private final ContextSnapshotFactory contextSnapshotFactory;

    @Autowired
    public Config(ObservationRegistry observationRegistry) {
        ObservationThreadLocalAccessor.getInstance()
                                      .setObservationRegistry(observationRegistry);
        contextSnapshotFactory = ContextSnapshotFactory.builder()
                                                       .contextRegistry(ContextRegistry.getInstance())
                                                       .clearMissing(true)
                                                       .build();
    }
    

    @Bean
    public ThreadPoolTaskExecutor someExecutor() {
        final ThreadPoolTaskExecutor taskExecutor =
                new TaskExecutorBuilder()/*...*/
                         .taskDecorator(runnable -> contextSnapshotFactory.captureAll(new Object[0]).wrap(runnable))
                         .build();
        taskExecutor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy());
        return taskExecutor;
    }
}

I'm not sure if this is a proper approach, or which one should be the suggested approach?

@marcingrzejszczak marcingrzejszczak added the bug A general bug label Aug 18, 2023
@marcingrzejszczak marcingrzejszczak added this to the 1.0.10 milestone Aug 18, 2023
@marcingrzejszczak
Copy link
Contributor

ObservationAwareSpanThreadLocalAccessor should be only attached if you want to use context propagation together with manually created spans. I removed the memory leak - now it should be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants