Skip to content

Add support for x-cloud-trace-context #248

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

Merged
merged 31 commits into from
Aug 13, 2020
Merged

Conversation

rob-spotify
Copy link
Contributor

Adds support for Google's x-cloud-trace-context header.

TODO Establish correct Span ID behaviour for when only a Trace ID is given in the header

@andrewhsu andrewhsu requested a review from rnburn August 5, 2020 00:09
@rob-spotify rob-spotify requested a review from rnburn August 6, 2020 17:45
parent_id[span_id_length] = s[offset];
++offset;
}
parent_id[span_id_length] = '\0';
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to parent_id.back() = '\0';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to undo this as it broke one of the tests - specifically one that send a short span ID of 123 - I think the null terminator was not being added on the end of the number i.e. 123\0 but at the end of the char array.

@rob-spotify rob-spotify requested a review from rnburn August 13, 2020 13:16
Copy link
Contributor

@rnburn rnburn left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you.

@rnburn rnburn merged commit 0253eba into lightstep:master Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants