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

is there a plan to change alibaba-dubbo dependency to apache-dubbo? #867

Closed
1 task done
geercode opened this issue Feb 20, 2019 · 41 comments
Closed
1 task done

is there a plan to change alibaba-dubbo dependency to apache-dubbo? #867

geercode opened this issue Feb 20, 2019 · 41 comments

Comments

@geercode
Copy link

  • Question
    since all projects have been transfered to apache incubator,is there any paln to support dubbo 2.7+ ?
    I have tried using dubbo-spring-boot-starter 2.7,the dependency of alibaba LoggerFactory is nologger supported.
@codefromthecrypt
Copy link
Member

codefromthecrypt commented Feb 20, 2019 via email

@geercode
Copy link
Author

I think dubbo will be at a unsatble circumstance for a very long time.Between 2.6 and 2.7,it's very much like just some packages' name changing of the same content.I suppose using osgi to make them work together is a better way.At the same time,dubbo is just under maintainance.The rest protocol which 2.6 is already supported is a key module.Before dubbo 3.0,there would not be too many developers would use it.

@codefromthecrypt
Copy link
Member

feel free to chat with me here I am ready to have a look at brave stuff soon https://gitter.im/openzipkin/zipkin

@codefromthecrypt
Copy link
Member

this is a problem as the code is very different.. 2.6.5 and 2.7.0 removed classes.. that can be ok but now we have to figure out what to do.

@codefromthecrypt
Copy link
Member

It appears the dubbo team are deciding whether to stop working with us based on the fact that will still support their old clients. I am very disappointed at this behavior. I hope they change https://lists.apache.org/thread.html/4ab3582245cb7daa79becaa7f644089f93da08b3a8ae0c3c90e89a1c@%3Cdev.dubbo.apache.org%3E

@codefromthecrypt
Copy link
Member

we can't break compat of the old dubbo users because we are nice. to figure out what to do can be easy or hard depending on bytecode compat. For example, grpc we were able to make a wide version range work because the core classes are still compatible from version 1.3.

However, I don't think this is the case with dubbo. version 2.7 seems not compatible with 2.6 unless you revert to reflection. this likely means we need a separate artifact.

For example, we can start brave-instrumentation-apache-dubbo or a different scheme like brave-instrumentation-dubbo-2.7 as the dubbo team also plan to break compatibility again in version 3.

The challenge here is thinking and planning only.. which route do we take? If so, which naming convention do we use? we will be stuck with it, so it is worthwhile having the thinking exercise.

@ralf0131
Copy link

Hi,

this is a problem as the code is very different.. 2.6.5 and 2.7.0 removed classes..

2.7.0 renames the package name to org.apache, but the goal is not to break the compatibility. Instead, the Dubbo community would like to maintain the compatibility as much as possible, but it is known that it is not 100% compatible. Yes, the community has received several reports of incompatibility. If there is an issue, please file it so the community would take a look and hopefully to fix it.

It appears the dubbo team are deciding whether to stop working with us based on the fact that will still support their old clients. I am very disappointed at this behavior. I hope they change https://lists.apache.org/thread.html/4ab3582245cb7daa79becaa7f644089f93da08b3a8ae0c3c90e89a1c@%3Cdev.dubbo.apache.org%3E

Not sure what you are talking about, Dubbo community never decide to stop working with some other community. Supporting open tracing does not mean stop working with zipkin.

For example, we can start brave-instrumentation-apache-dubbo or a different scheme like brave-instrumentation-dubbo-2.7 as the dubbo team also plan to break compatibility again in version 3.

The Dubbo community never plan to break compatibility again in Dubbo 3.x. Instead, compatibility should be always focused. So please stop speculating and feel free to discuss with the community via Email or Github issue.

@beiwei30
Copy link
Contributor

@geercode your particular issue may caused by dubbo-spring-boot-starter 2.7 itself instead of dubbo 2.7.0. Like @ralf0131 says, we always keep backward compatibility in mind. We will fully examine how dubbo 2.7 works well with Zipkin with no doubt. Stay tuned.

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Mar 25, 2019 via email

@codefromthecrypt
Copy link
Member

Also, I am glad that 3.0 will be binary compatible with 2.7. I mistakenly took "Before dubbo 3.0,there would not be too many developers would use it." that this wouldn't be the case

@codefromthecrypt
Copy link
Member

@ralf0131 ps are you hinting that we should be able to serve both alibaba (2.6.x) and apache (2.7) in the same classpath? I did try this early this month but ran into snags. It is good to know that it is considered a bug if we can't. This means we can consider co-hosting the instrumentation in the same artifact.

@codefromthecrypt
Copy link
Member

What I didn't report earlier this month was precisely the dubbo 2.7 and 2.6.6 compatibility problem. I incorrectly assumed it wasn't planned to work together in the same classpath. Right now they cannot co-exist, but I guess it is a bug.

problem 1: incompatible types returned from com.alibaba.dubbo.rpc.RpcContext.getContext()

    RpcContext rpcContext = RpcContext.getContext();

problem 2: incompatible types returned from org.apache.dubbo.common.Node.getUrl()

      isOneway = RpcUtils.isOneway(invoker.getUrl(), invocation);

The easiest way to see this problem is adding the following to instrumentation/dubbo/pom.xml

    <dependency>
      <groupId>org.apache.dubbo</groupId>
      <artifactId>dubbo</artifactId>
      <version>2.7.0</version>
      <optional>true</optional>
    </dependency>

@beiwei30 should this go into your issue then? or is it already solved?

@ralf0131
Copy link

Before dubbo 3.0,there would not be too many developers would use it

What @geercode stated here has nothing to do with the Apache Dubbo community and that statement is not true.

@ralf0131
Copy link

literally the opentracing thread includes rationale that "async call not work in zipkin plugin." this is what I mean. there was no issue raised here or support of any kind. Now, because I call it out some people now participate. I'm glad you are now, but it is strange to me that not only did you not see that there was a relationship between that thread and perceived problems with zipkin integration, but also find my mentioning anything about it out of line.

I've missed that message, I agree that this kind of issue should be reported to Zipkin community as well, and after some dig I found this issue is still waiting to be fixed. Maybe there should be some refactoring on the Zipkin plugin side in order to make it work.

@codefromthecrypt
Copy link
Member

ok thanks I feel maybe we are on the same page now.

As far as I can tell, we should re-try integration when dubbo 2.7.2 is out, or perhaps eagerly try once apache/dubbo#3576 is fixed.

If somehow this issue goes stale and above is fixed, please next person ping. I want this stuff to work and it seems promising if we are able to use the same module for multiple dubbo versions.

@ralf0131
Copy link

are you hinting that we should be able to serve both alibaba (2.6.x) and apache (2.7) in the same classpath? I did try this early this month but ran into snags. It is good to know that it is considered a bug if we can't. This means we can consider co-hosting the instrumentation in the same artifact.

No, I think the ideal behavior is that zipkin plugin should works without any change when runs on Dubbo 2.7.

@ralf0131
Copy link

problem 1: incompatible types returned from com.alibaba.dubbo.rpc.RpcContext.getContext()

    RpcContext rpcContext = RpcContext.getContext();

I think it is an issue from Dubbo side, which has not been address yet.

problem 2: incompatible types returned from org.apache.dubbo.common.Node.getUrl()

      isOneway = RpcUtils.isOneway(invoker.getUrl(), invocation);

I believe this is is a known issue, which has been fixed in the upcoming 2.7.1 release.

@codefromthecrypt
Copy link
Member

Thanks for the support @ralf0131. Let me know if you want me to raise an issue for problem 1, or if you have that already.

@beiwei30
Copy link
Contributor

What I didn't report earlier this month was precisely the dubbo 2.7 and 2.6.6 compatibility problem. I incorrectly assumed it wasn't planned to work together in the same classpath. Right now they cannot co-exist, but I guess it is a bug.

problem 1: incompatible types returned from com.alibaba.dubbo.rpc.RpcContext.getContext()

    RpcContext rpcContext = RpcContext.getContext();

problem 2: incompatible types returned from org.apache.dubbo.common.Node.getUrl()

      isOneway = RpcUtils.isOneway(invoker.getUrl(), invocation);

The easiest way to see this problem is adding the following to instrumentation/dubbo/pom.xml

    <dependency>
      <groupId>org.apache.dubbo</groupId>
      <artifactId>dubbo</artifactId>
      <version>2.7.0</version>
      <optional>true</optional>
    </dependency>

@beiwei30 should this go into your issue then? or is it already solved?

They have not been solved, but we will try to address it in release 2.7.2. It's no need to raise another issue, we will cross reference the current issue when we look into apache/dubbo#3728

@ralf0131
Copy link

ralf0131 commented Apr 9, 2019

Before dubbo 3.0,there would not be too many developers would use it

What @geercode stated here has nothing to do with the Apache Dubbo community and that statement is not true.

I just stated the fact normal developers would more like to use 2.6.4 to reach the most compatible mode.Your words just said dubbo is good, powerful, many ones were using it and would be a bunch men upgrading it to 2.7.0. I cannot agree with it.As I have seen now, I would like to upgrade it when the version 3.0 is coming out.Please dont dive in your colorful mind dreaming what you thought it was.I didn't mean duboo was not good.I mean you are rude and arrogant!You came here on the zipkin repo making your opinions over others.Why you alibaba modules change name all the time.Oh yeah I know,you are good name-makers."It's short for xxx, like a couch, like a plane, like jaguar mourning".
Shame for you as a chinese developer! @ralf0131

You can say what ever you like, that is your freedom. But please do not speak on behalf of the Apache Dubbo community and other Dubbo users, that is what I mean.

@codefromthecrypt
Copy link
Member

FYI, this is still awaiting 2.7.2

@ralf0131
Copy link

ralf0131 commented Jun 3, 2019

Hi, @adriancole In Dubbo 2.7.2 the compatibility problem should be fixed. Zipkin should work both on 2.6.x and 2.7.x without change. Could you help to verify it?

However when running on 2.7.x, there will be some extra overhead, Dubbo will convert com.alibaba classes into org.apache classes.

@codefromthecrypt
Copy link
Member

hi ... is 2.7.2 being voted on? I don't see it released yet https://search.maven.org/search?q=a:dubbo

We are getting ready to do our first release now, but I have no problem testing this for the next.

@ralf0131
Copy link

ralf0131 commented Jun 3, 2019

Hi,

Yes, Dubbo 2.7.2 is under release vote, but there is some trouble to get the link to staging repository ready. I will update once it is available, if you have time, it will be appreciated that you can test while the vote is ongoing.

@ralf0131
Copy link

ralf0131 commented Jun 3, 2019

@codefromthecrypt
Copy link
Member

sorry.. we had a lot of distractions over the last couple weeks. I tried what has been released and there are some compile concerns

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project brave-instrumentation-dubbo-rpc: Compilation failure: Compilation failure: 
[ERROR] /Users/acole/oss/incubator-zipkin-brave/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java:[29,41] error: cannot find symbol
[ERROR]   symbol:   class ExtensionLoader
[ERROR]   location: package com.alibaba.dubbo.common.extension
[ERROR] /Users/acole/oss/incubator-zipkin-brave/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java:[30,48] error: package com.alibaba.dubbo.config.spring.extension does not exist
[ERROR] /Users/acole/oss/incubator-zipkin-brave/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java:[31,42] error: cannot find symbol
[ERROR]   symbol:   class ResponseCallback
[ERROR]   location: package com.alibaba.dubbo.remoting.exchange
[ERROR] /Users/acole/oss/incubator-zipkin-brave/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java:[38,43] error: package com.alibaba.dubbo.rpc.protocol.dubbo does not exist
[ERROR] /Users/acole/oss/incubator-zipkin-brave/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java:[154,51] error: cannot find symbol
[ERROR]   symbol:   class ResponseCallback
[ERROR]   location: class TracingFilter
[ERROR] /Users/acole/oss/incubator-zipkin-brave/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java:[98,28] error: cannot find symbol
[ERROR]   symbol:   class FutureAdapter
[ERROR]   location: class TracingFilter
[ERROR] /Users/acole/oss/incubator-zipkin-brave/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java:[100,10] error: cannot find symbol
[ERROR]   symbol:   class FutureAdapter
[ERROR]   location: class TracingFilter
[ERROR] /Users/acole/oss/incubator-zipkin-brave/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java:[161,4] error: method does not override or implement a method from a supertype
[ERROR] /Users/acole/oss/incubator-zipkin-brave/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java:[165,4] error: method does not override or implement a method from a supertype
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :brave-instrumentation-dubbo-rpc

@cvictory
Copy link

I have write a sample that introduce your jar and dubbo-2.7.3-SNAPSHOT, and it work well.

But if you depend on dubbo-2.7 directly in your code, It cannot work for your test depend on some inner class of dubbo.
I think the code sourse still depend on dubbo-2.6.x , and add a module or integratation-test that depend on dubbo-2.7.3-SNAPSHOT to have a integration test.

image

This is my own project solution so that you can manage your source code with multiple dubbo verison.

@codefromthecrypt
Copy link
Member

ok so you are saying try again against snapshot huh? :D

@cvictory
Copy link

cvictory commented Jul 1, 2019

Yes, you can try to use snapshot. The release version will be published in this week later.

@codefromthecrypt
Copy link
Member

please advise as I'm still getting compile failure due to missing types

Screenshot 2019-07-03 at 8 39 11 PM

@beiwei30
Copy link
Contributor

beiwei30 commented Jul 4, 2019

@adriancole, pls. remove the imports for ExtensionLoader and SpringExtensionFactory since they appear only in javadoc.

We didn't realize ResponseCallback and FutureAdapter are used. We will consider to provide the compatible version in the next release. Alternatively, will you consider to support both versions by using reflection?

@beiwei30
Copy link
Contributor

beiwei30 commented Jul 4, 2019

@adriancole pls. check apache/dubbo@64aea16, it is part of 2.7.3 release

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Jul 4, 2019 via email

@chickenlj
Copy link

chickenlj commented Jul 4, 2019

Hi, I'll try to explain the current situation and the possible solution here:

  1. Zipkin continues to use com.alibaba.dubbo. packaged classes but wants to upgrade Dubbo dependency to 2.7.x.

    Because Dubbo refactored its package from com.alibaba.dubbo to org.apache.dubbo in 2.7.0, some Dubbo versions cannot seamlessly work with Zipkin without any change before 2.7.3.

    Start from 2.7.3, com.alibaba.dubbo.remoting.exchange.ResponseCallback and com.alibaba.dubbo.remoting.exchange.ResponseFuture will be supported in the compatible module. This means simply upgrade dependency version without any other code changes is possible.

  2. Zipkin can update its code to adapt to Dubbo's new package, please check the PR I submitted here Upgrade dubbo to 2.7.3 #938

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Jul 4, 2019 via email

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Jul 4, 2019 via email

@codefromthecrypt
Copy link
Member

So, I assume that the following constraint is ok with the dubbo team when considering upgrade paths.

  • If you use spring extension loader, com.alibaba:dubbo and org.apache.dubbo:dubbo are incompatible. you have to change your code to have this work.
  • If you are not using spring, you can bridge with deprecated apis until those are removed.

Is this correct?

codefromthecrypt pushed a commit that referenced this issue Jul 4, 2019
Starting with the 2.7.3 all of our main code can compile with
org.apache.dubbo:dubbo and also com.alibaba:dubbo

There is some strange work here because the spring integrations were not
backported. Until such time as it is, we need reflection to actually
load the extensions. As this is fragile, this change shouldn't merge
until a maven invoker test is setup to prove the tests also pass with
com.alibaba:dubbo still. Also, we need to cleanup documentation around
spring as it will mention types that don't exist.

Fixes #867
@codefromthecrypt
Copy link
Member

with some manual testing the code needed in the main tree can now work in 2.7.3

#939

The experience is less good because of code mentioned that isn't ported. It is really quite complex compared to any other integration. Anyway it is good progress.

codefromthecrypt pushed a commit that referenced this issue Aug 11, 2019
Starting with the 2.7.3 all of our main code can compile with
org.apache.dubbo:dubbo and also com.alibaba:dubbo

There is some strange work here because the spring integrations were not
backported. Until such time as it is, we need reflection to actually
load the extensions. As this is fragile, this change shouldn't merge
until a maven invoker test is setup to prove the tests also pass with
com.alibaba:dubbo still. Also, we need to cleanup documentation around
spring as it will mention types that don't exist.

Fixes #867
@codefromthecrypt
Copy link
Member

2.7.3 is out, but #939 are some problems it doesn't like how tests are executed and it is failing.

I'm beginning to think compatiliby is pointless on this, or at least a coal mine of effort and you can't tell if it works or not because tests can't execute. Regardless, I don't think we can afford to keep trying to get a compatible test working as every minor we try and then something doesn't work.

Maybe we should just give up create a separate artifact brave-instrumentation-dubbo. leave this one (brave-instrumentation-dubbo-rpc) alone as alibaba and after some point stop publishing it.

@codefromthecrypt
Copy link
Member

by the way, we have apache dubbo support, but only 2.7.4 which is not yet release passes tests. 2.7.3 fails for bytecode related problems, eventhough alibaba's latest dubbo is ok #1003

I hope this dubbo release vote finishes because it is really overdue getting this together, and we cannot rely on staging repositories

https://lists.apache.org/thread.html/95c0e433be7cfd9a3e36debc533064589ba64a0f192bde49ca0de4ab@%3Cdev.dubbo.apache.org%3E

@codefromthecrypt
Copy link
Member

We will put this into Brave 5.8 eventhough it is broken in travis until 2.7.4.

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

Successfully merging a pull request may close this issue.

6 participants