Skip to content
This repository has been archived by the owner on Dec 23, 2023. It is now read-only.

opencensus 0.28.1 breaks gRPC #2069

Closed
ejona86 opened this issue Dec 16, 2020 · 4 comments · Fixed by #2072
Closed

opencensus 0.28.1 breaks gRPC #2069

ejona86 opened this issue Dec 16, 2020 · 4 comments · Fixed by #2072
Labels

Comments

@ejona86
Copy link

ejona86 commented Dec 16, 2020

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);
                              ^
@ejona86 ejona86 added the bug label Dec 16, 2020
@steveniemitz
Copy link
Contributor

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.

ejona86 added a commit to ejona86/grpc-java that referenced this issue Dec 16, 2020
I didn't touch Protobuf and Netty; we upgrade those individually. Below
are issues I encountered that caused me to not upgrade (further).

Guava 30.1-android fails to build with Android without enabling
desugaring. google/guava#5358

Robolectric 4.4 breaks AndroidChannelBuilderTest.
grpc#7731

Opencensus 0.28.1+ is incompatible with gRPC.
census-instrumentation/opencensus-java#2069
grpc#7732

Truth now defines the asm dependency as "compile" although it is still
optional. But asm appears to have accidentally included incorrect gradle
module metadata in their release (I see they've disabled the metadata on
master) which make gradle think it requires Java 8. We could asm
everywhere, but that's is annoying. It seems likely this will resolve
itself.

Mockito can be upgraded to 3.4.0, but it deprecates initMocks, which
causes more code churn than I wanted in this commit. I still
synchronized the example versions on 3.4.0, though, as it was already
being used in some examples and the examples don't use initMocks.
ejona86 added a commit to grpc/grpc-java that referenced this issue Dec 17, 2020
I didn't touch Protobuf and Netty; we upgrade those individually. Below
are issues I encountered that caused me to not upgrade (further).

Guava 30.1-android fails to build with Android without enabling
desugaring. google/guava#5358

Robolectric 4.4 breaks AndroidChannelBuilderTest.
#7731

Opencensus 0.28.1+ is incompatible with gRPC.
census-instrumentation/opencensus-java#2069
#7732

Truth now defines the asm dependency as "compile" although it is still
optional. But asm appears to have accidentally included incorrect gradle
module metadata in their release (I see they've disabled the metadata on
master) which make gradle think it requires Java 8. We could asm
everywhere, but that's is annoying. It seems likely this will resolve
itself.

Mockito can be upgraded to 3.4.0, but it deprecates initMocks, which
causes more code churn than I wanted in this commit. I still
synchronized the example versions on 3.4.0, though, as it was already
being used in some examples and the examples don't use initMocks.
@gfelbing
Copy link

gfelbing commented Jan 7, 2021

I ran into the same issue, therefore handed in a PR to update opencensus in grpc-java: grpc/grpc-java#7787

@jsuereth
Copy link
Contributor

jsuereth commented Jan 7, 2021

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
Copy link
Author

ejona86 commented Jan 7, 2021

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

dfawley pushed a commit to dfawley/grpc-java that referenced this issue Jan 15, 2021
I didn't touch Protobuf and Netty; we upgrade those individually. Below
are issues I encountered that caused me to not upgrade (further).

Guava 30.1-android fails to build with Android without enabling
desugaring. google/guava#5358

Robolectric 4.4 breaks AndroidChannelBuilderTest.
grpc#7731

Opencensus 0.28.1+ is incompatible with gRPC.
census-instrumentation/opencensus-java#2069
grpc#7732

Truth now defines the asm dependency as "compile" although it is still
optional. But asm appears to have accidentally included incorrect gradle
module metadata in their release (I see they've disabled the metadata on
master) which make gradle think it requires Java 8. We could asm
everywhere, but that's is annoying. It seems likely this will resolve
itself.

Mockito can be upgraded to 3.4.0, but it deprecates initMocks, which
causes more code churn than I wanted in this commit. I still
synchronized the example versions on 3.4.0, though, as it was already
being used in some examples and the examples don't use initMocks.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants