talsma-ict / context-propagation Goto Github PK

This involves the following steps:

• Using a maven coverage tool such as jacoco
• Reporting to an online service such as coveralls.io
• Adding a CI coverage badge to the README file.

Showing how to use the span propagation.

• Make the build script also publish the released artifact on github:
  • github release API
  • example how to call from cli
  • Create and add pre-built artifacts to a GitHub release from CI server using the GitHub releases API
• Support release candidates
• Perform release versioning and tagging on release/xyz branch (like a quit mini git-flow)

Configure Travis CI to build this repository.

Expected Behavior

snapshot.reactivate() works from withing ForkJoinPool.commonPool() threads

Actual Behavior

snapshot.reactivate() throws exception:
Context manager "com.wirelesscar.componentbase.RequestContextManager" not found in service loader! Cannot reactivate: com.....

Java 9 has introduced more class loaders. It now exists three class loaders.
Bootstrap, Platform and System. https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html

ForkJoinPool.commonPool is created in a static block in ForkJoinPool with the Platform classloader,
that Thread pool will have Platform as context class loader.

Spring boot has it's LaunchedURLClassLoader that inherits from System class loader.

The cache in PriorityServiceLoader is using Thread.currentThread().getContextClassLoader() to load classes. This is not working with currentThread == commonPool.

I've verified that it works if I change line 108 in PriorityServiceLoader to read
final ClassLoader contextClassLoader = serviceType.getClassLoader();

I'm thinking of adding a check that does something similair to
contextClassLoader = Thread.currentThread().getContextClassLoader();
if (contextClassLoader.getName().contains("Platform") {
contextClassLoader = serviceType.getClassLoader();
}

My guess is that Jakarta EE etc still need to use the context class loader...

Steps to Reproduce the Problem

1. Create spring application that uses ContextAwareCompletableFuture
2. Compile and run it with java 11

Specifications

• Version: 1.0.3

• Platform: Linux

• Java version:
  openjdk version "11" 2018-09-25
  OpenJDK Runtime Environment 18.9 (build 11+28)
  OpenJDK 64-Bit Server VM 18.9 (build 11+28, mixed mode)
  --

• (If open source) Project link:

See talsma-ict/lazy4j#7 as reference.

In production code for a client, there seems to be a deadlock situation with the new 0.5.0 version of the ContextAwareCompletableFuture class in combination with calling thenApply and thenCombine in background threads.

I will create a new Pull Request for a unit test that reproduces the issue (and after that, attempt to fix it, obviously).

Is your feature request related to a problem? Please describe.

No problem, just extra code

Describe the solution you'd like

A new static method that returns a non-null Locale in any situation, falling back to the platform default locale if no LocaleContext is active.

Describe alternatives you've considered

n.a.

Additional context

Don't forget to update the example in the readme + update javadoc

Expected Behavior

ContextAwareCompletableFuture.thenCompose() works under Java 11 when the
composed CompletableFuture is created with supplyAsync or runAsync

Actual Behavior

A NoContextManagersFound exception is being thrown.

The problem is that ProjectAppliction.compose() is executed from the ForkJoinPool.commonPool
thread pool which is using the "System" class loader (jdk.internal.loader.ClassLoaders$AppClassLoader)
instead of The class loader that the Spring Boot application has loaded all classes into (org.springframework.boot.loader.LaunchedURLClassLoader).
The ContextAwareCompletableFuture.supplyAsync() within ProjectAppliction.compose() will then try to create a new snapshot and load managers from the "wrong" classloader.

My current solution is to inject a ThreadFactory into ForkJoinPool.commonPool. The attached project can be built with -Pworkaround to enable this. Is has to do some tricks with maven antrun plugin to inject a class into the jar....

The only viable but somewhat ugly fix for this that I have come up with is to add thenCompose() methods that takes a BiFunction<? super T, ContextSnapshot, ? extends CompletionStage>. or take a Function<ValueWithContext<? super T>, ? extends CompletionStage>.

That way you can pass the ContextSnapshot to ContextAwareCompletableFuture.supplyAsync() to avoid the call to createContextSnapshot() and therefor avoid the ServiceLoader trying to load classes in the wrong classloader.

There is also a fix that is using a ThreadLocal to pass the ContextSnapshot between the ContextAwareCompletableFuture that executes the thenCompose() and any ContextAware code that needs to call ContextManagers.createContextSnapshot from within the method that theCompose calls,
that is ugly in another way and probably has problems in it's own.

Steps to Reproduce the Problem

1. Unzip attached project
2. mvn install && java -jar target/project-1.0-SNAPSHOT.jar
3. Notice that exception is being thrown from the call to supplyAsync at ProjectApplication.java:52

Specifications

• Version: 1.0.4
• Platform: Any
• Java version: OpenJDK 64-Bit Server VM 18.9 (build 11+28, mixed mode)
• (If open source) Project link: test-project-2.zip

Is your feature request related to a problem? Please describe.
In response to bug reports, ContextAwareCompletableFuture has been modified to take a snapshot at the end of each completion stage, to be propagated into a 'next' stage.
Often, there is no 'next' stage. In this case a relatively expensive context snapshot was created but never used.

Describe the solution you'd like
Think of a mechanism that can be used to propagate the original snapshot at creation time through each stage, or have alternative methods when a new snapshot should be taken.

For example: thenApplyAndTakeSnapshot(Function) ..

Describe alternatives you've considered
Alternative is to leave things as they are and not worry about the additional snapshots being created.

Additional context
This request relates to documenting the java 8 code, and issues #50, #54 , #55, #57

Test that reproduces the NoSuchElementException that gets thrown:

    @Test
    public void testConcurrentSnapshots() throws ExecutionException, InterruptedException {
        int threadcount = 25;
        ExecutorService threadpool = Executors.newFixedThreadPool(threadcount);
        try {
            List<Future<ContextSnapshot>> snapshots = new ArrayList<Future<ContextSnapshot>>(threadcount);
            for (int i = 0; i < threadcount; i++) {
                snapshots.add(threadpool.submit(new Callable<ContextSnapshot>() {
                    public ContextSnapshot call() throws Exception {
                        return ContextManagers.createContextSnapshot();
                    }
                }));
            }

            for (int i = 0; i < threadcount; i++) {
                assertThat(snapshots.get(i).get(), is(notNullValue()));
            }
        } finally {
            threadpool.shutdown();
        }
    }

Results in:

java.util.NoSuchElementException
	at sun.misc.CompoundEnumeration.nextElement(CompoundEnumeration.java:59)
	at java.util.ServiceLoader$LazyIterator.hasNextService(ServiceLoader.java:357)
	at java.util.ServiceLoader$LazyIterator.hasNext(ServiceLoader.java:393)
	at java.util.ServiceLoader$1.hasNext(ServiceLoader.java:474)
	at nl.talsmasoftware.context.ContextManagers.createContextSnapshot(ContextManagers.java:80)

Create a maven module to update old artifacts with relocation information.

See https://help.github.com/articles/creating-an-issue-template-for-your-repository/

Showing how to use the servletrequest propagation.

Expected Behavior

linking from e.g. java8 module to ContextManager should work

Actual Behavior

It doesn't. It points to a github link that doesn't work.

Steps to Reproduce the Problem

1. Read the javadoc 😉

Specifications

Try to supply the correct links/link configuration to the maven Javadoc plugin.

Rationale:

• Opentracing is already in the package nl.talsmasoftware.context.opentracing
• The name SpanManager is more natural and now no longer overloaded from OpenTracing itself
• We can keep the old class for 1 release as a deprecated empty subclass implementation.

Is your feature request related to a problem? Please describe.
When a context gets activated or deactivated, there may be other parties interested.
For instance, observing a current user context activation may be useful to set the user's name in a certain MDC field.

Describe the solution you'd like
A SPI to register observers to specific context managers.
Maybe even a SPI to register observers to any (all) context manager.

Describe alternatives you've considered
Wrapping contexts: leads to needless objects and mixed concerns.
Custom 'delegating' context manager: managers that are not really in control of the objects they supposedly manage.

Additional context
Not yet sure on how to tackle this.

Expected Behavior

Maven version badge

Actual Behavior

Maven inaccessible badge

Possibly maven central went https only?

Is your feature request related to a problem? Please describe.

Kotlin coroutines are stored in memory and have their own contexts and can be scheduled to resume on different threads.

ThreadLocal contexts won't work with coroutines out of the box.

Describe the solution you'd like

Support coroutines similar to the solution that was created for Slf4J MDC context: kotlinx-coroutines-slf4j

Describe alternatives you've considered

Additional context

• Documentation: https://github.com/Kotlin/kotlinx.coroutines/blob/master/docs/coroutine-context-and-dispatchers.md#thread-local-data

Is your feature request related to a problem? Please describe.

Obtaining the current Context allows code to close the context that was not responsible for initializing it. Closing a context is the responsibility of the one creating it.

Describe the solution you'd like

Deprecate ContextManager.getActiveContext() and add a new ContextManager.getActiveContextValue().

Describe alternatives you've considered

We could leave things the way they are, but this change makes it a little harder to 'do the wrong thing'.

Additional context

Similar change will happen in opentracing ScopeManager in version 0.32.0.

Describe the solution you'd like

Add support for https://micrometer.io similar to the current dropwizard-metrics
module.

Describe alternatives you've considered

Obviously the metrics library.

Additional context

Investigate the possibilities to support micrometer out of the box with an add-on module.

Is your feature request related to a problem? Please describe.

Grpc Context offers a fluent api (starting from the static Context class).
Let's see if it could be fully supported as well from this context-propagation library and how much effort it would cost.

Describe the solution you'd like

Clean grpc-context propagation without hacks or workarounds using a delegating ContextManager.

Describe alternatives you've considered

none, I just realised this was not even on the radar yet due to lots of other work

Additional relevant information

Is your feature request related to a problem? Please describe.

No problem, maybe a more standard solution available.

Describe the solution you'd like

If possible replace my context-propagation with a standard solution.

Describe alternatives you've considered

This comment pointed me to MicroProfile Context Propagation.
Is may be a suitable substitute to my ContextManagers. It's a more standard approach anyway.

Additional relevant information

Perhaps create a default bridge implementation to the microprofile standard, which would possibly save us a lot of maintenance.

Is your feature request related to a problem? Please describe.

Java 8 allows for cleaner code because:

• lambda's
• Optional
• Functional interfaces; i.e. lose our SnapshotSupplier in favour of Supplier<Snapshot>.

Describe the solution you'd like

The next major release will be a breaking change, so use that chance for improvement.

1. Set minimum java version to 1.8
2. Include the -java8 module into the main version and remove it from the build
3. Replace SnapshotSupplier by Supplier<Snapshot>.
4. Make optional results return Optional
5. Remove all deprecated code
6. Change the main package so the code can co-exist with v1
7. Add a compatibility module containing a 'bridge' to v1 ContextManager implementations
8. Change the ContextManager: replace getActiveContext by Optional<T> getActiveContextValue (breaking change)

Describe alternatives you've considered

'Hard' breaking changes (leaving out the last two items from the list) but that may hurt current users unnecessarily.
Two-step getActiveContext --> getActiveContextValue through deprecation plus introducing new method, unfortunately introducing a new method in an interface is only possible compatibly from Java8's default methods.

Additional context

I decide to test some more. 😃

Given the common fork pool has parallelism = 3.
If you run this a couple of times you will see that Foo is always present on all three threads in the first part, as expected.
But on the second part, it is sometimes present on the thread 2 and 3, sometimes not.

I guess my question is, what is the scope of the context? When is it deleted?

  public static void main(String[] args) throws InterruptedException {
    System.out.println(ForkJoinPool.commonPool());
    ContextAwareCompletableFuture
        .runAsync(() -> new DummyContextManager().initializeNewContext("Foo"))
        .thenCompose(aVoid1 -> {
          ContextAwareCompletableFuture<Void> future1 = ContextAwareCompletableFuture.runAsync(Main2::check);
          ContextAwareCompletableFuture<Void> future2 = ContextAwareCompletableFuture.runAsync(Main2::check);
          ContextAwareCompletableFuture<Void> future3 = ContextAwareCompletableFuture.runAsync(Main2::check);
          return CompletableFuture.allOf(future1, future2, future3);
        })
        .join();

    System.out.println("-");
    ContextAwareCompletableFuture.supplyAsync(() -> "")
        .thenCompose(aVoid1 -> {
          ContextAwareCompletableFuture<Void> future1 = ContextAwareCompletableFuture.runAsync(Main2::check);
          ContextAwareCompletableFuture<Void> future2 = ContextAwareCompletableFuture.runAsync(Main2::check);
          ContextAwareCompletableFuture<Void> future3 = ContextAwareCompletableFuture.runAsync(Main2::check);
          ContextAwareCompletableFuture<Void> future4 = ContextAwareCompletableFuture.runAsync(Main2::check);
          ContextAwareCompletableFuture<Void> future5 = ContextAwareCompletableFuture.runAsync(Main2::check);
          return CompletableFuture.allOf(future1, future2, future3, future4, future5);
        })
        .join();
  }

  private static void check() {
    try {
      Thread.sleep(100);
    } catch (InterruptedException e) {
      e.printStackTrace();
    }
    DummyContextManager.currentValue().ifPresent(s1 -> System.out.println(s1 + " " + Thread.currentThread().getName()));
  }

}

When wrapping a resulting CompletableFuture into a new ContextAwareCompletableFuture, no reference to the original, wrapped future is kept. This makes cancellation impossible through the wrapped result.

The opentracing module serves two purposes:

1. Propagate spans accross threads
2. Trace context switches

With a client I discovered that the traces became incredibly complex if each context-switch introduces two spans (capturing and reactivating the snapshot).
Because the propagation of spans is massively more important than tracing the context switches, let's make that second part optional (activated by a static method call and/or an environment variable).

Reminder on the additional complexity of async requests:

Have a look at https://github.com/opentracing-contrib/java-web-servlet-filter (thanks @pavolloffay !)

Use animal sniffer to guarantee java 5 source level (and JVM class-use!) compatiblity.
Then we can at least use java 8 lambda's in unit tests etc.

Add a simple example for ContextAwareCompletableFuture. I tried with this code but did not work as I expected. Maybe I'm missing something?

public class Main {
    public static void main(String[] args) throws InterruptedException {
        ContextAwareCompletableFuture
                .supplyAsync(() -> {
                    try {
                        Thread.sleep(500);
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                    return "FooBar";
                })
                .thenAccept(s -> new DummyContextManager().initializeNewContext(s))
                .thenCompose((aVoid) -> {
                    ContextAwareCompletableFuture<Void> ret = new ContextAwareCompletableFuture<>();
                    Thread thread = new Thread(() -> {
                        //CurrentValue is empty.
                        //Can it propagate to this new thread?
                        System.out.println("New thread " + DummyContextManager.currentValue());
                        ret.complete(null);
                    });
                    thread.start();
                    return ret;
                })
                //Same thing here
                .thenAccept((s1) -> System.out.println(DummyContextManager.currentValue()));

        Thread.sleep(3000);
    }

}

CompletableFuture has couple of static methods to create CompletableFuture instances.
ContextAwareCompletableFuture overrides a few of these today. runAsync() and supplyAsync().

ContextAwareCompletableFuture should also override completedFuture(U) and implement
failedFuture(Throwable) that was introduced in Java 9. Maybe even introduce a context-propagation-java9 module that implements all methods of java 9 version of CompletableFuture.

ContextAwareCompletableFuture.completedFuture(U) is a priority as it is easy to misunderstand it today and think that it returns a ContextAwareCompletableFuture when it actually is returning a standard CompletableFuture where the Async methods do not copy contexts.

Similar to SLF4J's MDC, Log4j2 has the concept of a ThreadContext (Log4j2 manual). This ThreadContext acts both as MDC and Nested Diagnostic Context (NDC).

Creating a snapshot should use:

• ThreadContext.getContext()
• ThreadContext.getImmutableStack()

Reactivating a snapshot could then use Log4j2's CloseableThreadContext which is able to revert the changes on its own:

1. Call either CloseableThreadContext.putAll(...) or CloseableThreadContext.pushAll(...) (with ContextStack.asList())
2. Then call either CloseableThreadContext.Instance.putAll(...) or CloseableThreadContext.Instance.pushAll(...) (with ContextStack.asList()) on the returned CloseableThreadContext.Instance

Then when closing the Context you can call CloseableThreadContext.Instance.close() to revert the changes.

The naming of the existing SLF4J MDC library (mdc-propagation) might be unfortunate. For Log4j2 I would propose including the library name instead, e.g. log4j2-propagation. Also note that just using log4j in the name without the "2" might lead to confusion since Log4j1 also has a MDC and NDC, however Log4j1 is no longer supported so adding support for it here might not be needed.

I just came across your library and have not used it, but if you want I can try creating a pull request.

SpanManager.currentSpan(): Span was replaced by:
SpanManager.current(): ManagedSpan.

Similar to TracerResolver, context propagation also caches java ServiceLoader instances for re-use.

See opentracing-contrib/java-tracerresolver#31 why this may give problems in complex classloader configurations.

The following should be done:

• Move the current 'ContextManagers.Loader' concept to its own (final, package-protected) class.
• Replace current reflection mechanism by compiled code with catch-LinkageErrors construct.
• Add support for @Priority annotation while we're at it.

To add and enforce license headers in all source files.

This requires something like:

interface Contextualizable extends AutoCloseable {}
interface ContextManager<T> {
  Contextualizable initializeNewContext(T value);
  T getCurentContextualValue()
}
final class ContextManagers {
  public static ContextSnapshot createContextSnapshot() { ... }
}
final class ContextSnapshot {
  public AutoCloseable reactivate() { ...}
}

And a SPI definition where context manager implementations can be registered.

The removal of the ServiceLoader cache has a large impact on the performance of this library.
ServiceLoader is only caching found providers in one instance, creating a new ServiceLoader on every call to ContextManagers.createContextSnapshot() is triggering a full classpath scanning.

We are not running in any application server, we run a plain Spring Boot 2.1 application.

My suggestion is that ServiceLoader caching is made configurable or that the providers that ServiceLoader finds are cached in a hash with the class loder as key or similar solution where ServiceLoader only has to be called once.

This can call clear() on all context managers that support clearing their context from the current thread.

Note: Adding clear() to the ContextManager interface would mean a breaking change.
I can add a separate interface e.g. Clearable that can be implemented by all context managers that support it and make the change non-breaking. This will increase the documentation burden to explain how this works however.

For more up to date testframework

See

https://travis-ci.org/talsma-ict/context-propagation/builds/331185437#L2896

Build with the same maven version everywhere using the maven wrapper.

Same as: talsma-ict/umldoclet#285

span can be null in ScopeContext if OpentracingSpanManager is used with
ContextAware* when there is no active span.
The problem is that ContextManagers.createContextSnapshot() only stores activeContext.getValue() which is null
ContextManagers.reactivate() then retreives null from the snapshot and calls OpentracingSpanManger.initializeNewContext(null)

The test in ScopeContext.close only checks that closed is false before calling span.close.

The fix could be to set closed to true in initializeNewContext() if span is null,
or add a nullcheck in SpanContext.close.

So the release is not dependent on the developer's laptop with SSH + GPG keys and passwords.

• introduce Travis CI build user
• add maven wrapper
• configure travis build
• configure release script
• sort out credentials in travis

This applies to the ContextAwareCompletableFuture that implements CompletionStage.

Semantically, these stages are dependent on each-other. Therefore, it would be more logical to also propagate context changes from one stage to the next.
The current implementation of ContextAwareCompletableFuture propagates the initial context snapshot into all following stages however.

In retrospect, this may not be the most natural propagation mechanism.

Although technically a breaking change, I would classify changing this as as a bug fix.

The current intended behaviour is to propagate the initial snapshot across all stages and don't attempt to change the context in the background threads. Admittedly, this behaviour is not explicitly documented at the moment.
When contexts are not updated in completion stage functions, the fix will not have any impact. Where they are changed, the change may have impact on user code.

Thanks @nosslin579 for posting this question that made me think a bit more about completion stages

For documentation purposes, sometimes people with to depend on the javadoc classifier for certain projects. Using the bill-of-materials, these versions can also be declared.

Normally this is not a problem for transitive dependencies, but still nice to have.