openzipkin / zipkin-aws

Reporters and collectors for use in Amazon's cloud
Apache License 2.0
69 stars 34 forks source link

Ensures X-Ray json has parent_id if type=segment #63

Closed tabdulradi closed 6 years ago

tabdulradi commented 6 years ago

I got the following error from X-Ray agent

2017-12-04T15:00:37Z [Error] unprocessed segment: {
  ErrorCode: "MissingParentId",
  Id: "7a0ac3a355fe232e",
  Message: "Invalid subsegment. ErrorCode: MissingParentId, Cause: null"
}
2017-12-04T15:00:37Z [Warn] {"trace_id":"1-7381dc2c-6825ddf77a0ac3a355fe232e","id":"7a0ac3a355fe232e","type":"subsegment","name":"my-service","start_time":1.512399636124057E9,"end_time":1.512399636125315E9,"annotations":{"akka_actor_class":"com.bamtechmedia.opentracing.cinnamon.xray.examples.cinnamon.Greeter","akka_actor_system-message":"Create(None)","component":"akka.actor","span_kind":"akka.actor.system-message.receive"}}

The json document seems to have "type":"subsegment" but no parent_id, which is illegal.

This PR ensures we either write both fields or none of them.

codefromthecrypt commented 6 years ago

thanks for the help! can you rebase this over master as #59 is in which adds a test class. Would be nice to have a test for this.

tabdulradi commented 6 years ago

I figured out my changes are also wrong. parent_id is optional, unless type=subsegment, in which case it becomes required.

I ended up writing my own zipkin to xray conversion logic. I defined classes that represents XRay model, to ensure I can't construct an illegal json. The code is here, supports Zipkin and Jaeger, but it is written in Scala https://github.com/tabdulradi/opentracing-xray

tabdulradi commented 6 years ago

To fix this PR, we should always write the parent_id if it not null (restore line 54). But don't write the type=subsegment unless parentId exists.

Unfortunately, I don't have the time to update this PR. You can close it if no one wants to own it.