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

geercode commented 5 years ago

[x] 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 commented 5 years ago

in general we dont have plans unless users ask for something. are you able to help migrate this to apache dubbo? is it possible to make it work with both libraries (we dont want to break people who haven't moved yet)

On Wed, Feb 20, 2019, 7:26 PM jerryniu <notifications@github.com wrote:

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.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openzipkin/brave/issues/867, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD612uQruAsqEmcybPx9Gdsd3hXv9mOks5vPTDrgaJpZM4bFAIf .

geercode commented 5 years ago

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 commented 5 years ago

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

codefromthecrypt commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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.

On Mon, Mar 25, 2019 at 4:30 PM Huxing Zhang notifications@github.com wrote:

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.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/openzipkin/brave/issues/867#issuecomment-476099125, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD619tPWCsSuJSuR3HB1YgcnkveDU3Vks5vaIkSgaJpZM4bFAIf .

codefromthecrypt commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 https://github.com/apache/incubator-dubbo/issues/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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 https://github.com/apache/incubator-dubbo/issues/3728

ralf0131 commented 5 years ago

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 commented 5 years ago

FYI, this is still awaiting 2.7.2

ralf0131 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Hi,

The link to staging repo: https://repository.apache.org/content/repositories/orgapachedubbo-1026/

codefromthecrypt commented 5 years ago

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 commented 5 years ago

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.

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

codefromthecrypt commented 5 years ago

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

cvictory commented 5 years ago

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

codefromthecrypt commented 5 years ago

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

beiwei30 commented 5 years ago

@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 commented 5 years ago

@adriancole pls. check https://github.com/apache/dubbo/commit/64aea16c0eceb9e3cf2576259c36f415259a3ded, it is part of 2.7.3 release

codefromthecrypt commented 5 years ago

will check when next online. thanks

On Thu, Jul 4, 2019, 10:47 AM Ian Luo notifications@github.com wrote:

@adriancole https://github.com/adriancole pls. check apache/dubbo@ 64aea16 https://github.com/apache/dubbo/commit/64aea16c0eceb9e3cf2576259c36f415259a3ded, it is part of 2.7.3 release

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openzipkin/brave/issues/867?email_source=notifications&email_token=AAAPVVYWXNUBYZEOV7OFRL3P5VQCTA5CNFSM4GYUAIP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZGFCIA#issuecomment-508317984, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAPVV3MGZX6U5YYDMUSBHTP5VQCTANCNFSM4GYUAIPQ .

chickenlj commented 5 years ago

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

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.
Zipkin can update its code to adapt to Dubbo's new package, please check the PR I submitted here https://github.com/openzipkin/brave/pull/938

codefromthecrypt commented 5 years ago

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

here:

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.

agree

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.

we need to test this separately from any other changes, but seems correct. To verify we need to have an invoker test that also runs tests against alibaba package as for a few versions so far people think everything ok but not yet. No offense, just this is required in order to say something is supported.

Zipkin can update its code to adapt to Dubbo's new package, please check the PR I submitted here #938 https://github.com/openzipkin/brave/pull/938

This will force a break which is not a good first step, even if that can be an acceptable second step.

codefromthecrypt commented 5 years ago

Note: there's also another missing type

com.alibaba.dubbo.common.beanutil.JavaBeanDescriptor

We need to make our integration tests conditionally use this and whatever the apache replacement is. Similarly, we need replacement javadoc for this:

/**

Notes for com.alibaba:dubbo
{@code com.alibaba.dubbo.common.extension.ExtensionLoader} supplies the tracing implementation
which must be named "tracing". For example, if using the
{@code com.alibaba.dubbo.config.spring.extension.SpringExtensionFactory}, only a bean named
"tracing" will be injected. */

codefromthecrypt commented 5 years ago

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 commented 5 years ago

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

https://github.com/openzipkin/brave/pull/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 commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

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

openzipkin / brave

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

Notes for com.alibaba:dubbo