Skip to content
This repository has been archived by the owner on Aug 30, 2022. It is now read-only.

Do not strip leading zeros from trace IDs and span IDs #259

Merged
merged 3 commits into from
Jan 29, 2021
Merged

Do not strip leading zeros from trace IDs and span IDs #259

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e36d09
Do not strip leading zeros from trace IDs
tobiasstadler Jan 29, 2021
cff57dc
Do not strip leading zeros from span IDs
tobiasstadler Jan 29, 2021
b5a7328
high should only be used if it is non zero
tobiasstadler Jan 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/jaegertracing/SpanContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,9 @@ class SpanContext : public opentracing::SpanContext {
void print(Stream& out) const
{
_traceID.print(out);
out << ':' << std::hex << _spanID << ':' << std::hex << _parentID << ':'
<< std::hex << static_cast<size_t>(_flags);
out << ':' << std::setw(16) << std::setfill('0') << std::hex << _spanID
<< ':' << std::setw(16) << std::setfill('0') << std::hex << _parentID
<< ':' << std::hex << static_cast<size_t>(_flags);
}

void ForeachBaggageItem(
Expand Down
2 changes: 1 addition & 1 deletion src/jaegertracing/SpanContextTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ TEST(SpanContext, testFormatting)
SpanContext spanContext(TraceID(255, 255), 0, 0, 0, SpanContext::StrMap());
std::ostringstream oss;
oss << spanContext;
ASSERT_EQ("ff00000000000000ff:0:0:0", oss.str());
ASSERT_EQ("00000000000000ff00000000000000ff:0000000000000000:0000000000000000:0", oss.str());
}

TEST(SpanContext, testBaggage)
Expand Down
9 changes: 2 additions & 7 deletions src/jaegertracing/TraceID.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,8 @@ class TraceID {
template <typename Stream>
void print(Stream& out) const
{
if (_high == 0) {
out << std::hex << _low;
}
else {
out << std::hex << _high << std::setw(16) << std::setfill('0')
<< std::hex << _low;
}
out << std::setw(16) << std::setfill('0') << std::hex << _high
Copy link
Member

Choose a reason for hiding this comment

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

this does not seem backwards compatible, what's the reason for removing if (_high == 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.

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.

I though the trace id is now always 32 hex chars long. But I was wrong.

Copy link
Member

Choose a reason for hiding this comment

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

No, all Jaeger SDKs still use 64bit by default, you have to explicitly enable 128bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, noticed that after your comment. I misread the prs for the other sdks.

<< std::setw(16) << std::setfill('0') << std::hex << _low;
}

uint64_t high() const { return _high; }
Expand Down
2 changes: 1 addition & 1 deletion src/jaegertracing/TraceIDTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ TEST(TraceID, testPrint)
{
std::ostringstream oss;
oss << TraceID(0, 10);
ASSERT_EQ("a", oss.str());
ASSERT_EQ("0000000000000000000000000000000a", oss.str());
}

} // namespace jaegertracing