Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 2 commits into from

Conversation

tabdulradi
Copy link

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
Copy link
Member

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.

@@ -51,11 +51,13 @@
.append(span.traceId(), 0, 8)
.append('-')
.append(span.traceId(), 8, 32).toString());
if (span.parentId() != null) writer.name("parent_id").value(span.parentId());
writer.name("id").value(span.id());
if (span.kind() == null
|| span.kind() != Span.Kind.SERVER && span.kind() != Span.Kind.CONSUMER) {
Copy link
Author

@tabdulradi tabdulradi Dec 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adriancole I am not sure why span.kind() has to affect whether the trace is subsegment or not. I think we should only check for parentId. i.e move the block on line 57 outside this block.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In brave all spans have a parent except for the root. A span of type Server or Consumer would be a Segment in X-Ray terms because these are the initial span in a service

@tabdulradi
Copy link
Author

tabdulradi commented Jan 24, 2018

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-Ray AWS X-Ray
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants