Add support for span links to Jaeger export. #1251
Conversation
deejgregor
force-pushed
the
feature-jaeger-links
branch
from
5237358 to
65c1e7a
Compare
|
One important note: I'm not really a C++ developer, so please look over this with a fine-toothed comb. |
| reference.__set_spanId(*(reinterpret_cast<const int64_t *>(span_context.span_id().Id().data()))); | ||
| #endif | ||
|
|
||
| references_.push_back(reference); |
There was a problem hiding this comment.
how will this list of links get exported to the backend? Does some code need to be added in ThriftSender::Append() to do that?
There was a problem hiding this comment.
Good catch! I missed a line when I pulled the changes over. Added: https://github.com/open-telemetry/opentelemetry-cpp/pull/1251/files#diff-3fc4e4d4cb58766986df79016e29275bf3712c2fdcda74c598152f42598bf546R43
Codecov Report
@@ Coverage Diff @@
## main #1251 +/- ##
=======================================
Coverage 92.31% 92.31%
=======================================
Files 198 198
Lines 7281 7281
=======================================
Hits 6721 6721
Misses 560 560 |
| std::vector<thrift::Tag> Tags() noexcept { return std::move(tags_); } | ||
| std::vector<thrift::Tag> ResourceTags() noexcept { return std::move(resource_tags_); } | ||
| std::vector<thrift::Log> Logs() noexcept { return std::move(logs_); } | ||
| std::vector<thrift::SpanRef> References() noexcept { return std::move(references_); } |
There was a problem hiding this comment.
Seems SpanRefs are not attached to jaeger_span?
There was a problem hiding this comment.
Yup! I goofed and missed bringing over a line. Added: https://github.com/open-telemetry/opentelemetry-cpp/pull/1251/files#diff-3fc4e4d4cb58766986df79016e29275bf3712c2fdcda74c598152f42598bf546R43
(and made sure nothing else is missing)
deejgregor
force-pushed
the
feature-jaeger-links
branch
from
65c1e7a to
c4cb6e3
Compare
deejgregor
force-pushed
the
feature-jaeger-links
branch
from
c4cb6e3 to
c4f9021
Compare
|
Just force pushed c4f9021 with clang-format fixes for what was identified in https://github.com/open-telemetry/opentelemetry-cpp/runs/5474270958?check_suite_focus=true. Force pushed a fix earlier for the missing line in #1251 (comment) |
3 participants
Fixes # (issue)(no existing issue found)Changes
Add support for span links to Jaeger export.
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes[ ] Changes in public API reviewed(no API changes)