census-instrumentation / opencensus-java

A stats collection and distributed tracing framework
https://opencensus.io
Apache License 2.0
672 stars 201 forks source link

opencensus 0.28.1 breaks gRPC #2069

Closed ejona86 closed 3 years ago

ejona86 commented 3 years ago

0.28.1 hides io.opencensus.trace.unsafe.ContextUtils which is used directly by gRPC. This was broken by #2059. It seems ContextUtils should be marked deprecated for the moment, instead of hidden entirely. This breakage is especially surprising given the change was in a point release.

Migrating to ContextHandleUtils will need to take some careful thought, as there are multiple ways it would break with the current gRPC integration. Even if we fix this today in gRPC, it'd still be a month before the next release. In addition, a flag-day change like this requires upgrading an entire application all-at-once and so may prevent people from upgrading grpc and getting on the new opencensus APIs; keeping the old class available allows upgrading a piece at a time without breaking the world.

CC @zoercai

grpc-java/census/src/main/java/io/grpc/census/CensusTracingModule.java:42: error: ContextUtils is not public in io.opencensus.trace.unsafe; cannot be accessed from outside package
import io.opencensus.trace.unsafe.ContextUtils;
                                 ^
grpc-java/census/src/main/java/io/grpc/census/CensusTracingModule.java:344: error: ContextUtils is not public in io.opencensus.trace.unsafe; cannot be accessed from outside package
      return ContextUtils.withValue(context, span);
             ^
grpc-java/census/src/main/java/io/grpc/census/CensusTracingModule.java:385: error: ContextUtils is not public in io.opencensus.trace.unsafe; cannot be accessed from outside package
          newClientCallTracer(ContextUtils.getValue(Context.current()), method);
                              ^
steveniemitz commented 3 years ago

I ran into this a couple days ago when I tried to upgrade to 0.28.1, it breaks pretty much the entire google-cloud-* ecosystem since they all use gRPC.

gfelbing commented 3 years ago

I ran into the same issue, therefore handed in a PR to update opencensus in grpc-java: https://github.com/grpc/grpc-java/pull/7787

jsuereth commented 3 years ago

Hey, sorry for the breakage! I can revert this visibility change and issue a 0.28.x release with this fix (although let me first check and make sure it doesn't break things any further). In the meantime, are you able to use 0.27.x?

ejona86 commented 3 years ago

gRPC tests appeared to be passing fine with 0.28.0, so I think just 0.28.1 is a problem.