context: pluggable Storage mechanism. #2461

zhangkun83 · 2016-11-29T22:23:19Z

Currently Context propagate in-thread by its own ThreadLocal, and
cross-thread propagation must be done with the mechanisms provided by
gRPC Context. However, there are frameworks (e.g. what we are using
inside Google) which have already established context-propagation
mechanisms. If gRPC Context may ride on top of them, it would be
propagated automatically without additional effort from the application.

The Storage API allows gRPC Context to be attached to anything. The
default implementation still uses the ThreadLocal. If an override
implementation is present, gRPC Context will use it.

Currently Context propagate in-thread by its own ThreadLocal, and cross-thread propagation must be done with the mechanisms provied by gRPC Context. However, there are frameworks (e.g. what we are using inside Google) which have already established context-propagation mechanisms. If gRPC Context may ride on top of them, it would be propagated automatically without additional effort from the application. The Storage API allows gRPC Context to be attached to anything. The default implementation still uses the ThreadLocal. If an override implementation is present, gRPC Context will use it.

ejona86 · 2016-12-02T00:15:17Z

context/src/main/java/io/grpc/Context.java

- * previously bound context. For example:
+ * <p>A Context object can be {@link #attach attached} to the {@link Storage}, which effectively
+ * forms a <b>scope</b> for the context.  The scope is typically bound to the current thread, though
+ * alternative {@link Storage} implementations can alter this behavior.  Within a scope, its Context


I think Context becomes useless if it isn't bound in some way to the current thread. The scope has no meaning then, since it is basically undefined. All the use cases we're considering now are all thread-local-like storage.

Makes sense. Removed the possibility of non-thread-locals.

ejona86 · 2016-12-02T00:16:55Z

context/src/main/java/io/grpc/Context.java

+ * <p>A Context object can be {@link #attach attached} to the {@link Storage}, which effectively
+ * forms a <b>scope</b> for the context.  The scope is typically bound to the current thread, though
+ * alternative {@link Storage} implementations can alter this behavior.  Within a scope, its Context
+ * is accessible even across API boundaries, through {@link #current}.  The scope can later be


s/can later be/is later/

ejona86 · 2016-12-02T00:22:21Z

context/src/main/java/io/grpc/Context.java

+
+  // For testing
+  static Storage storage() {
+    if (storage == null) {


Use a static initializer? Seems simpler.

private static Storage storage = createStorage();

I don't mind continuing to use the storage() method.

In cases it would throw, using a method for lazy initialization allows each attempt to use Context would throw an informative exception. With static initializer, only the first use would throw the informative exception, and subsequent uses would throw some generic "class not found". The former is a bit more friendly to users.

note that in similar stuff (like log4j2), a fallback context is used on error. This logs at bootstrap, but doesn't render the system useless.. judgement call if this is the right choice, but anyway there's prior art here.

ps something like this could be fun to test using @raphw's byte buddy https://github.com/raphw/byte-buddy
seems like you could set the current classloader to some child, and load a crappy context storage into that (which you created with byte buddy). Then set the classloader back in a finally (after you've tested your errors work as should)

note that in similar stuff (like log4j2), a fallback context is used on error. This logs at bootstrap, but doesn't render the system useless.. judgement call if this is the right choice, but anyway there's prior art here.

We (@ejona86 and I) believe it's better to fail loudly if an override class is present and it fails to load. In our internal use case, failure to load the override would break the expected automatic context propagation, in which case breaking the system early is superior than letting it run with impaired functionality.

ejona86 · 2016-12-02T00:25:51Z

context/src/main/java/io/grpc/Context.java

+   * alternative implementation named {@code io.grpc.ContextStorageOverride} exists in the
+   * classpath, it will be used instead of the default implementation.
+   *
+   * <p>This API is <a href="https://github.com/grpc/grpc-java/issues/2462">experimental</a> and


Hehe. We can't use the annotation.

zhangkun83

Thanks @ejona86 !

zhangkun83 · 2016-12-02T01:57:20Z

context/src/main/java/io/grpc/Context.java

- * previously bound context. For example:
+ * <p>A Context object can be {@link #attach attached} to the {@link Storage}, which effectively
+ * forms a <b>scope</b> for the context.  The scope is typically bound to the current thread, though
+ * alternative {@link Storage} implementations can alter this behavior.  Within a scope, its Context


Makes sense. Removed the possibility of non-thread-locals.

zhangkun83 · 2016-12-02T01:57:38Z

context/src/main/java/io/grpc/Context.java

+ * <p>A Context object can be {@link #attach attached} to the {@link Storage}, which effectively
+ * forms a <b>scope</b> for the context.  The scope is typically bound to the current thread, though
+ * alternative {@link Storage} implementations can alter this behavior.  Within a scope, its Context
+ * is accessible even across API boundaries, through {@link #current}.  The scope can later be


zhangkun83 · 2016-12-02T02:02:50Z

context/src/main/java/io/grpc/Context.java

+
+  // For testing
+  static Storage storage() {
+    if (storage == null) {


In cases it would throw, using a method for lazy initialization allows each attempt to use Context would throw an informative exception. With static initializer, only the first use would throw the informative exception, and subsequent uses would throw some generic "class not found". The former is a bit more friendly to users.

codefromthecrypt · 2016-12-04T04:47:23Z

We ***@***.*** <https://github.com/ejona86> and I) believe it's better to fail loudly if an override class is present and it fails to load. In our internal use case, failure to load the override would break the expected automatic context propagation, in which case breaking the system early is superior than letting it run with impaired functionality.

reasonable choice. just helping smoke it out :P

zhangkun83 assigned ejona86 Nov 29, 2016

Make Storage API experimental.

137bf69

ejona86 approved these changes Dec 2, 2016

View reviewed changes

Fix documentation.

1073b37

zhangkun83 commented Dec 2, 2016

View reviewed changes

zhangkun83 merged commit 8486286 into grpc:master Dec 2, 2016

This was referenced Dec 4, 2016

Support multiple child spans from the same thread, aka variable trace shapes openzipkin/brave#166

Closed

Standard(s) for in-process propagation opentracing/specification#23

Open

lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019

context: pluggable Storage mechanism. #2461

context: pluggable Storage mechanism. #2461

Uh oh!

Conversation

zhangkun83 commented Nov 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhangkun83 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codefromthecrypt commented Dec 4, 2016 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants