opendiffy / diffy

Other
1.26k stars 142 forks source link

Is there any documentation for thrift testing #1

Closed hmenghouzz closed 6 years ago

hmenghouzz commented 6 years ago

Hi,

Is there any documentation for using diffy on Thrift comparison? I couldn't find any Thrift related examples from README. I have some thrift IDLs. I used apache thrift code generator to generate the Java files. Then used "javac" and "jar" command to combine those java class files into a single JAR archive file.

Tried to run diffy with this command ... -service.protocol=thrift \ -allowHttpSideEffects=true \ -enableThriftMux='true' \ -thrift.jar='/path/to/thrift.jar' \ -thrift.serviceClass=$SomeServiceName

Then I got such error: Error in custom provider, ai.diffy.lifter.ThriftLifter$InvalidServiceException: Invalid service: ....

I noticed that the filenames returned empty in this function: https://github.com/opendiffy/diffy/blob/master/src/main/scala/ai/diffy/proxy/ThriftDifferenceProxy.scala#L37

Must the fat jar that feeds diffy be generated by Scrooge?

Really appreciate your time here ๐Ÿ˜Š

opendiffy commented 6 years ago

The Scrooge AST generator used in Diffy looks for all original .thrift files (not the generated code). Can you explode the jar and verify that the .thrift files are included.

hmenghouzz commented 6 years ago

Thank you for helping out ๐Ÿ‘ One more quick question. Now Diffy loaded the thrift file successfully. But got some error during thrift parsing:

1) Error in custom provider, com.twitter.scrooge.frontend.FileParseException: Exception parsing: base.thrift
  at ai.diffy.proxy.DifferenceProxyModule$.providesDifferenceProxy(DifferenceProxy.scala:24)
  at ai.diffy.proxy.DifferenceProxyModule$.providesDifferenceProxy(DifferenceProxy.scala:24)
  while locating ai.diffy.proxy.DifferenceProxy
1 error
...
    at ai.diffy.Main.main(Main.scala)
Caused by: com.twitter.scrooge.frontend.FileParseException: Exception parsing: base.thrift
    at com.twitter.util.NoStacktrace(Unknown Source)
Caused by: com.twitter.scrooge.frontend.TypeNotFoundException: SomeSeed
    9:optional SomeSeed someSeed,

code snippets from base.thrift showed below

struct Session {
     ...
    9:optional SomeSeed someSeed,
    ...
}
struct SomeSeed {
    1:optional string seedOne,
    2:optional string seedTwo,
}

I won't see the TypeNotFoundException if Session structure defined after SomeSeed got defined. i.e.

struct SomeSeed {
    1:optional string seedOne,
    2:optional string seedTwo,
}
struct Session {
     ...
    9:optional SomeSeed someSeed,
    ...
}

Is this a bug in diffy?

sn126 commented 6 years ago

This is a limitation of the Scrooge library we use to parse the thrift into AST. The definition of a struct is required to be parsed before it is nested into another struct.

puneetkhanduri commented 6 years ago

@hmenghouzz Would you be open to contributing these learnings as documentation? I think we often miss the necessary level of detail in our documentation when we write the code. Your help is much appreciated.

hmenghouzz commented 6 years ago

@puneetkhanduri Yes. I will update the documentation after I solving the thrift parsing issue. :grinning:

hmenghouzz commented 6 years ago

@sn126 @opendiffy I saw some parsing error com.twitter.scrooge.frontend.FileParseException: Exception parsing: xxx.thrift when ran Diffy. Scrooge doesn't seem to support circular dependencies so far? Do you have any plan to support it in the near future?

struct TFoo {
  1. required i32 some_id, 
  2: optional TBar bar,
}

struct TBar {
  1: required i32 some_id,
  2: optional TFoo foo,
}

struct TT{
   1: optional list<TT> listValue
}
puneetkhanduri commented 6 years ago

Thrift does not support circular dependencies. This is not a scrooge limitation.

hmenghouzz commented 6 years ago

@puneetkhanduri "cyclic structs - Starting with version 0.9.2, Thrift supports structs that contain themselves, or other structs to be declared later." ๐Ÿ˜‰ I got this information from https://thrift.apache.org/docs/features

puneetkhanduri commented 6 years ago

You are right. Also, scrooge does support thrift 0.10.0 so we need to bump the library versions in diffy. I gave it a quick try but ran into some dependency conflicts. I can resolve this in a couple of days abut feel free to take a shot. What you have to do is bump the scrooge generator and core versions to 18.5.0 in build.sbt and then resolve dependency conflicts that arise from that change. Once the conflicts are resolved, you should be good to go.

puneetkhanduri commented 6 years ago

@hmenghouzz mind giving it a shot now. I have bumped the libraries but I think we still need some special handling of recursive structs within Diffy.

hmenghouzz commented 6 years ago

@puneetkhanduri thank you so much for bumped the version of scrooge. Good News, I loaded the thrift jar which contains a bunch of cyclic structs. No exception was thrown.

However, I got some other issues when I ran Diffy:

  1. Sbt assembly will throw deduplicate-exception java.lang.RuntimeException: deduplicate: different file contents found in the following:
    7 errors were encountered during merge
    java.lang.RuntimeException: deduplicate: different file contents found in the following:
    /Users/hmeng/.ivy2/cache/io.netty/netty-codec-http/jars/netty-codec-http-4.1.16.Final.jar:META-INF/io.netty.versions.properties
    /Users/hmeng/.ivy2/cache/io.netty/netty-codec/jars/netty-codec-4.1.16.Final.jar:META-INF/io.netty.versions.properties
    ...

    I modified the assemblyMergeStrategy in build.sbt to solve this.

    
    libraryDependencies ++= Seq(
    ...
    "org.slf4j" % "slf4j-simple" % "1.6.4"
    ) ++ finatraDependencies ++ testDependencies.map(_ % "test"),

assemblyMergeStrategy in assembly := { case "BUILD" => MergeStrategy.discard case PathList("scala", "tools", *) => MergeStrategy.last case PathList("META-INF", xs @ *) => MergeStrategy.discard case _ => MergeStrategy.last }


2. I'm still confused about how to setup diffy running environment when **-service.protocol=thrift**
 a) I have set up a local port forwarding to a valid remote thrift server port
 b) my nodejs code can talk to the remote thrift server (localhost: 9990) successfully
```javascript
var thrift = require('thrift');
var connection = thrift.createHttpConnection(thrift_host, thrift_port, {
    path: '/Entry.php',
    transport: thrift.TBufferedTransport,
    protocol: thrift.TBinaryProtocol,
}),

var multiplexer = new thrift.Multiplexer();
var goodService = require('./gen-nodejs/GoodService');
var goodServiceClient = multiplexer.createClient('goodService', goodService.Client, connection);
goodServiceClient.getSomething((...), function (err, message) {...});

c) Then I changed the thrift_host and thrift_port to diffy-proxy port (localhost: 8880) and rerun the thrift call. But no data showed in diffy report webpage.

To be honest I never used java or scala before. I'm going to dig into the project as much as I can to debug it.

xinglinyin commented 6 years ago

Is the problem about diffy support thrift solved? I am also trying. @hmenghouzz @puneetkhanduri

puneetkhanduri commented 6 years ago

The last library bump and the build problems highlighted by @hmenghouzz have been fixed and merged. Please feel free to add to this issue if you are having any problems using thrift with diffy.

xinglinyin commented 6 years ago

I changed the thrift_host and thrift_port to diffy-proxy port (localhost: 8880) and rerun the thrift call. But no data showed in diffy report webpage.

the same to me.
image

xinglinyin commented 6 years ago

@hmenghouzz @puneetkhanduri. how can i debug it. Looks like the diff thrift proxy server didn't receive any request.

puneetkhanduri commented 6 years ago

Can you share your diffy server logs. There should be some errors in it.

puneetkhanduri commented 6 years ago

@xinglinyin Can you also share how you are creating a thrift client to send thrift requests to your diffy instance.

puneetkhanduri commented 6 years ago

@hmenghouzz I just noticed that you are using thrift over http - this is currently not supported. Diffy currently only supports thrift over tcp.

xinglinyin commented 6 years ago

@puneetkhanduri
First of all, thank you very much for your prompt reply. I send thrift request by company unified pressure measurement platformใ€‚ then Directly sent to the target thrift server is ok. but not work for diffy proxy.

the diffy server logs: image image

xinglinyin commented 6 years ago

@puneetkhanduri
How do I configure diffy rightly? I don't need send thrift request over http. I means i configure diffy incorrectly. image

xinglinyin commented 6 years ago

my .thrift file is like this: image

xinglinyin commented 6 years ago

the diffy server no reaction at all when I send thrift request to it. image

xinglinyin commented 6 years ago

@puneetkhanduri
I'm creating a python thrift client to send thrift requests to my diffy instance like this:

socket = TSocket.TSocket('10.20.154.164', 8884)

socket = TSocket.TSocket('10.6.26.236', 33941)

transport = TTransport.TBufferedTransport(socket)
protocol = TBinaryProtocol.TBinaryProtocol(transport)
client = TianchiService.Client(protocol)
socket.setTimeout(10 * 1000)
transport.open()
go_resp = client.GetAdModelByAdids(req)
xinglinyin commented 6 years ago

@puneetkhanduri I really need your help. I've been trying to debug the source code, but I've just started learning scala. It's hard for a beginner. thank you very much again.

hmenghouzz commented 6 years ago

@puneetkhanduri @xinglinyin I didn't work on that for a long time. And actually I couldn't get the diff result since we are using http connection. Please keep me updated : )

xinglinyin commented 6 years ago

@hmenghouzz you means that it's the bug of diffy? thrift request used the http connection?

hmenghouzz commented 6 years ago

@xinglinyin it's not a bug. We just use different transport ways. I don't have bandwidth to work on this currently. I think you can sync up with puneetkhanduri. He might be the right person to help.

xinglinyin commented 6 years ago

image

the console do'n print the "xinglin dudu come here".

puneetkhanduri commented 6 years ago

@xinglinyin : It might be easier to jump on hangouts(puneet.khanduri@gmail.com) or skype(puneet_khanduri). Feel free to ping me on either and we can troubleshoot your installation together.

puneetkhanduri commented 6 years ago

Also, I just added back logging support that we had lost while doing the last library bump. Your logs should be more verbose now.

xinglinyin commented 6 years ago

@puneetkhanduri
I got the detail debug info such as: image

xinglinyin commented 6 years ago

@puneetkhanduri
When are you free๏ผŸ Can I contact you through Hangouts?

puneetkhanduri commented 6 years ago

You are exceeding the netty frame size limit of 2.14 GB. I suspect you are not actually creating Thrift objects that large and that this is thrift multiplexing error on the client side. To confirm this hypothesis, can you run your diffy instance with the following option "-enableThriftMux=false". can then use a non-multiplexing thrift client and server(for your application). If you are not using thrift multiplexing in your application then just turning it off in Diffy with that command should be sufficient.

puneetkhanduri commented 6 years ago

This issue was resolved offline and the changes were merged to master.

RuixiaZhang commented 5 years ago

Hi,

Is there any documentation for using diffy on Thrift comparison? I couldn't find any Thrift related examples from README. I have some thrift IDLs. I used apache thrift code generator to generate the Java files. Then used "javac" and "jar" command to combine those java class files into a single JAR archive file.

Tried to run diffy with this command ... -service.protocol=thrift -allowHttpSideEffects=true -enableThriftMux='true' -thrift.jar='/path/to/thrift.jar' -thrift.serviceClass=$SomeServiceName

Then I got such error: Error in custom provider, ai.diffy.lifter.ThriftLifter$InvalidServiceException: Invalid service: ....

I noticed that the filenames returned empty in this function: https://github.com/opendiffy/diffy/blob/master/src/main/scala/ai/diffy/proxy/ThriftDifferenceProxy.scala#L37

Must the fat jar that feeds diffy be generated by Scrooge?

Really appreciate your time here ๐Ÿ˜Š

I encounter the same question with you, but the .thrift file have been included in the jar ,so I would like to ask you how can I fix this issue. Thank you.

Hi,

Is there any documentation for using diffy on Thrift comparison? I couldn't find any Thrift related examples from README. I have some thrift IDLs. I used apache thrift code generator to generate the Java files. Then used "javac" and "jar" command to combine those java class files into a single JAR archive file.

Tried to run diffy with this command ... -service.protocol=thrift -allowHttpSideEffects=true -enableThriftMux='true' -thrift.jar='/path/to/thrift.jar' -thrift.serviceClass=$SomeServiceName

Then I got such error: Error in custom provider, ai.diffy.lifter.ThriftLifter$InvalidServiceException: Invalid service: ....

I noticed that the filenames returned empty in this function: https://github.com/opendiffy/diffy/blob/master/src/main/scala/ai/diffy/proxy/ThriftDifferenceProxy.scala#L37

Must the fat jar that feeds diffy be generated by Scrooge?

Really appreciate your time here ๐Ÿ˜Š

I have encounter the same issues with you: 1) Error in custom provider, ai.diffy.lifter.ThriftLifter$InvalidServiceException: Invalid service: UserService at ai.diffy.proxy.DifferenceProxyModule$.providesDifferenceProxy(DifferenceProxy.scala:24) (via modules: com.google.in ject.util.Modules$OverrideModule -> ai.diffy.proxy.DifferenceProxyModule$)

image So, I would like to ask you how can I solve this issue and the diffy works. Really appreciate your time here too~

RuixiaZhang commented 5 years ago

docker run -i -t -p 8880:8880 -p 8881:8881 -p 8888:8888 -v /tmp:/tmp diffy/diffy \ -candidate=ip:9405 \ -master.primary=ip:9405 \ -master.secondary=ip:9405 \ -service.protocol=thrift \ -allowHttpSideEffects=true \ -enableThriftMux='true' \ -thrift.jar='/tmp/nameofjar.jar' \ -serviceName="$ServiceName" \ -proxy.port=:8880 \ -admin.port=:8881 \ -http.port=:8888 \ -rootUrl=localhost:8888

jar is included in the directory of tmp, and include all the files of .thrift