Jaeger: Auto-detect compact vs. binary Thrift formats in jaeger-agent UDP servers

[yurishkuro]

Requirement - what kind of business use case are you trying to solve?

Only open one UDP port on jaeger-agent, to minimize attack surface.

Problem - what in Jaeger blocks you from solving the requirement?

Agent currently uses distinct ports (https://github.com/jaegertracing/documentation/pull/262) for different Thrift formats:

Port | Protocol | Function
---- | ------- | ---
6831 | UDP | accept jaeger.thrift in compact Thrift protocol used by most current Jaeger clients
6832 | UDP | accept jaeger.thrift in binary Thrift protocol used by Node.js Jaeger client (because thriftrw npm package does not support compact protocol)

Proposal - what do you suggest to solve the problem or improve the existing situation?

Perhaps it's possible to detect the format by inspecting a few bytes in the header of the message.

[chandresh-pancholi]

@yurishkuro could you please help me out here where to start?

[yurishkuro]

The bytes received from udp are decoded here: https://github.com/jaegertracing/jaeger/blob/master/cmd/agent/app/processors/thrift_processor.go

This processor is designed to work with a single thrift format (TProtocol), so will require some creative refactoring. But before that it would be good to just experiment with sending compact and binary payloads (eg using go and Node clients) and see if it's possible to tell which is which from the packet header.

[chandresh-pancholi]

It was very difficult for me to find out protocol from binary payload in binary protocol.
As for compact, the first byte 81 is a compact protocol id.
I suggest putting the if-else check.

if(byte == 82) {
// compact protocol
} else {
// binary protocol
}

@yurishkuro what's your suggestion?

[yurishkuro]

we need to know how the first byte is interpreted in the binary protocol, i.e. is there a chance that it may be also equal to 82

[chandresh-pancholi]

We can figure out Protocols by reading bytes. Below is the code.
In the binary protocol, I am reading the first 4 bytes which are significant to understand the protocol.
In compact, first byte is good enough to figure out the protocol.

func (s *ThriftProcessor) isBinaryProtocol(payload []byte)  {
    buf := payload[0:4]

    value := int32(binary.BigEndian.Uint32(buf))
    version := int64(int64(value) & thrift.VERSION_MASK)
    if version == thrift.VERSION_1{
        s.logger.Info("binary protocol," zap.Any("version", version))
    }

}

func (s *ThriftProcessor) isCompactProtocol(payload []byte) int  {
    buf := bytes.NewBuffer(payload[0:1])

    protocolId, err := buf.ReadByte()
    if err != nil{
        s.logger.Error("read byte buffer failed",  zap.Error(err))
    }

    if protocolId == thrift.COMPACT_PROTOCOL_ID {
        s.logger.Info("protocolId", zap.Any("proto", protocolId))
    }

    return thrift.COMPACT_PROTOCOL_ID
}

[yurishkuro]

Did you check if we're using "strict writes" in binary? because in non-strict mode binary writes struct name without any prefix.

If strict write is confirmed, then the check seems to be even simpler, binary starts with 0x80 (first byte of VERSION_1 = 0x80010000), compact with 0x82 (COMPACT_PROTOCOL_ID).

[chandresh-pancholi]

Yes, Jaeger binary protocol uses thrift.NewTBinaryProtocolFactoryDefault() which default sets strictWrite true

builder.go L64

var (
    protocolFactoryMap = map[Protocol]thrift.TProtocolFactory{
        compactProtocol: thrift.NewTCompactProtocolFactory(),
        binaryProtocol:  thrift.NewTBinaryProtocolFactoryDefault(),
    }
)

func NewTBinaryProtocolFactoryDefault() *TBinaryProtocolFactory {
    return NewTBinaryProtocolFactory(false, true)
}

[chandresh-pancholi]

@yurishkuro Could you suggest a way for going forward? We can use the above logic to run it on 1 port. We can deduce the protocol type if run it in single port (e.g. 6831).

What do you think?

[yurishkuro]

Yes, it looks like we can do that. Do you want to create a pull request?

We already integration-test clients with the backend:

https://github.com/jaegertracing/jaeger/blob/6cb35708ca5fe04e32f7a5ea1493760fa0dfdcb8/crossdock/docker-compose.yml#L8-L12