How to use jaeger-thrift classifier thrift92 right?

[candyleer]

My project thrift version is 0.5.0,so it's conflict with jaeger-thrift,so I follow the docs to use classifier thrift92,but when i replace the pom ,I found it's not compatible with jaeger-core.
because the jaeger-core UdpSender is dependent on org.apache.thrift.protocol.TCompactProtocol,
org.apache.thrift.protocol.TProtocolFactory ,and in class com.uber.jaeger.agent.thrift.Agent.Client#Client(org.apache.thrift.protocol.TProtocol),it's constructor use original thrift,how can i use org.shadow.apache.thrift92 (classifier thrift92 supplied ),not org.apache.thrift to avoid conflict? rewrite the UdpSender and ThriftUdpTransport and any relation class that dependent on org.apache.thrift ?

[vprithvi]

Thanks for creating this issue, it seems that this functionality is broken.

jaeger-core depends on thrift, but does so by directly accessing a transitive dependency of jaeger-thrift. This is an dependency abstraction leak.

A reasonable way of fixing this would be to move all the thrift related code (ThriftSender and subclasses) into jaeger-thrift, and have jaeger-core depend only on the exposed interfaces from jaeger-thrift.

@yurishkuro Any objections to moving this code? (It breaks backwards compatibility with people directly instantiating the Sender because they would have to update the import statement)

[yurishkuro]

what does the current (relevant) dependency tree look like in terms of classes?

I agree that it should be jaeger-core -> jaeger-thrift, but perhaps at the class level we can fix it without breaking the API.

[vprithvi]

For jaeger-core, it looks like the following

I don't think we can easily fix it at the class level, mainly because by doing so, we'd still have jaeger-core directly dependending on thrift.

It looks like we can maintain backward compatibility by having the classes in jaeger-core proxy to the new location of the classes in jaeger-thrift and mark the old classes deprecated. People would get compile warnings, and can migrate to the new location of the classes on their own timeline.

[yurishkuro]

in core we have UdpSender extends ThriftSender (same for HttpSender)

The simplest backwards compatible fix I see is to do this:

• move all 3 classes (Udp, Http, Thrift senders) to jaeger-thrift (in different package)
• re-create UdpSender/HttpSender classes in jaeger-core as thin extensions of the respective classes in jaeger-thrift

[vprithvi]

Yes, this is exactly what I'm suggesting, I'll have a PR up in a bit.

[candyleer]

any plan on this issue?

[vprithvi]

Apologies, we had prioritized other things, I'll get back to this sometime this week.
That being said, we are happy to accept pull requests if you are willing to put one up!

[jlazevedo]

Hi there, I’m also having issues with thrift versions and unfortunately the fixes introduced with v0.27.0-RC1 were not enough :(

Exception Details:
  Location:
    io/jaegertracing/senders/UdpSender.<init>(Ljava/lang/String;II)V @52: invokeinterface
  Reason:
    Type 'io/jaegertracing/thrift/reporters/protocols/ThriftUdpTransport' (current frame, stack[4]) is not assignable to 'org/apache/thrift/transport/TTransport'
  Current Frame:
    bci: @52
    flags: { }
    locals: { 'io/jaegertracing/senders/UdpSender', 'java/lang/String', integer, integer }
    stack: { 'io/jaegertracing/senders/UdpSender', uninitialized 40, uninitialized 40, 'org/apache/thrift/protocol/TProtocolFactory', 'io/jaegertracing/thrift/reporters/protocols/ThriftUdpTransport' }
  Bytecode:
    0x0000000: 2ab2 0004 1db7 0005 2bc6 000a 2bb6 0006
    0x0000010: 9a00 0612 024c 1c9a 0007 111a af3d 2a2b
    0x0000020: 1cb8 0007 b500 082a bb00 0959 2ab4 000a
    0x0000030: 2ab4 0008 b900 0b02 00b7 000c b500 0db1
    0x0000040:                                        
  Stackmap Table:
    full_frame(@19,{Object[#1],Object[#59],Integer,Integer},{})
    same_frame(@22)
    same_frame(@30)

After running missing-link I found another related with ThriftSender:

[WARNING] Category: Method being called not found
[WARNING]   In artifact: io.jaegertracing:jaeger-core:0.27.0-RC1
[WARNING]     In class: io.jaegertracing.senders.UdpSender
[WARNING]       In method:  <init>(java.lang.String, int, int):58
[WARNING]       Call to: io.jaegertracing.agent.thrift.Agent$Client.<init>(org.apache.thrift.protocol.TProtocol)
[WARNING]       Problem: Method not found: io.jaegertracing.agent.thrift.Agent$Client.<init>(org.apache.thrift.protocol.TProtocol)
[WARNING]       Found in: io.jaegertracing:jaeger-thrift:0.27.0-RC1
[WARNING]       --------
[WARNING]     In class: io.jaegertracing.senders.ThriftSender
[WARNING]       In method:  calculateProcessSize(io.jaegertracing.thriftjava.Process):85
[WARNING]       Call to: io.jaegertracing.senders.ThriftSender.getSize(org.apache.thrift.TBase)
[WARNING]       Problem: Method not found: io.jaegertracing.senders.ThriftSender.getSize(org.apache.thrift.TBase)
[WARNING]       Found in: io.jaegertracing:jaeger-core:0.27.0-RC1
[WARNING]       --------
[WARNING]       In method:  calculateSpanSize(io.jaegertracing.thriftjava.Span):93
[WARNING]       Call to: io.jaegertracing.senders.ThriftSender.getSize(org.apache.thrift.TBase)
[WARNING]       Problem: Method not found: io.jaegertracing.senders.ThriftSender.getSize(org.apache.thrift.TBase)
[WARNING]       Found in: io.jaegertracing:jaeger-core:0.27.0-RC1
[WARNING]       --------
[WARNING]
[WARNING] Category: FIELD NOT FOUND
[WARNING]   In artifact: io.jaegertracing:jaeger-core:0.27.0-RC1
[WARNING]     In class: io.jaegertracing.senders.UdpSender
[WARNING]       In method:  <init>(java.lang.String, int, int):58
[WARNING]       Access to: io.jaegertracing.senders.UdpSender.protocolFactory
[WARNING]       Problem: Field not found: protocolFactory
[WARNING]       Found in: io.jaegertracing:jaeger-core:0.27.0-RC1

Thank you so much for the effort! Really helpful! I'll try to find some time to reciprocate with a PR. ;)

Cheers

[objectiser]

Re-opening to deal with additional dependencies on apache thrift.