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

Commit 405ded8

Browse files
isaachierisaachier
authored
Fix usage of propagation headers config (#91)
* Fix usage of propagation headers config Signed-off-by: Isaac Hier <[email protected]> * Add more propagation fixes and coverage Signed-off-by: Isaac Hier <[email protected]> * Improve coverage for propagation tests Signed-off-by: Isaac Hier <[email protected]>
1 parent ab6e3a6 commit 405ded8

File tree

5 files changed

+118
-29
lines changed

5 files changed

+118
-29
lines changed

‎src/jaegertracing/SpanContext.h‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -164,19 +164,24 @@ class SpanContext : public opentracing::SpanContext {
164164
forEachBaggageItem(f);
165165
}
166166

167-
bool operator==(const SpanContext& rhs) const
167+
friend bool operator==(const SpanContext& lhs, const SpanContext& rhs)
168168
{
169169
{
170-
std::lock(_mutex, rhs._mutex);
171-
std::lock_guard<std::mutex> lock(_mutex, std::adopt_lock);
170+
std::lock(lhs._mutex, rhs._mutex);
171+
std::lock_guard<std::mutex> lhsLock(lhs._mutex, std::adopt_lock);
172172
std::lock_guard<std::mutex> rhsLock(rhs._mutex, std::adopt_lock);
173-
if (_baggage != rhs._baggage) {
173+
if (lhs._baggage != rhs._baggage) {
174174
return false;
175175
}
176176
}
177-
return _traceID == rhs._traceID && _spanID == rhs._spanID &&
178-
_parentID == rhs._parentID && _flags == rhs._flags &&
179-
_debugID == rhs._debugID;
177+
return lhs._traceID == rhs._traceID && lhs._spanID == rhs._spanID &&
178+
lhs._parentID == rhs._parentID && lhs._flags == rhs._flags &&
179+
lhs._debugID == rhs._debugID;
180+
}
181+
182+
friend bool operator!=(const SpanContext& lhs, const SpanContext& rhs)
183+
{
184+
return !operator==(lhs, rhs);
180185
}
181186

182187
private:

‎src/jaegertracing/Tracer.h‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,13 @@ class Tracer : public opentracing::Tracer,
9898
config.sampler().makeSampler(serviceName, *logger, *metrics));
9999
std::shared_ptr<reporters::Reporter> reporter(
100100
config.reporter().makeReporter(serviceName, *logger, *metrics));
101-
return std::shared_ptr<Tracer>(new Tracer(
102-
serviceName, sampler, reporter, logger, metrics, options));
101+
return std::shared_ptr<Tracer>(new Tracer(serviceName,
102+
sampler,
103+
reporter,
104+
logger,
105+
metrics,
106+
config.headers(),
107+
options));
103108
}
104109

105110
~Tracer() { Close(); }
@@ -217,6 +222,7 @@ class Tracer : public opentracing::Tracer,
217222
const std::shared_ptr<reporters::Reporter>& reporter,
218223
const std::shared_ptr<logging::Logger>& logger,
219224
const std::shared_ptr<metrics::Metrics>& metrics,
225+
const propagation::HeadersConfig& headersConfig,
220226
int options)
221227
: _serviceName(serviceName)
222228
, _hostIPv4(net::IPAddress::localIP(AF_INET))
@@ -225,9 +231,9 @@ class Tracer : public opentracing::Tracer,
225231
, _metrics(metrics)
226232
, _logger(logger)
227233
, _randomNumberGenerator()
228-
, _textPropagator()
229-
, _httpHeaderPropagator()
230-
, _binaryPropagator()
234+
, _textPropagator(headersConfig, _metrics)
235+
, _httpHeaderPropagator(headersConfig, _metrics)
236+
, _binaryPropagator(_metrics)
231237
, _tags()
232238
, _restrictionManager(new baggage::DefaultRestrictionManager(0))
233239
, _baggageSetter(*_restrictionManager, *_metrics)

‎src/jaegertracing/TracerTest.cpp‎

Lines changed: 66 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@
1414
* limitations under the License.
1515
*/
1616

17+
#include "jaegertracing/Tracer.h"
1718
#include "jaegertracing/Config.h"
1819
#include "jaegertracing/Constants.h"
1920
#include "jaegertracing/Span.h"
2021
#include "jaegertracing/SpanContext.h"
2122
#include "jaegertracing/Tag.h"
2223
#include "jaegertracing/TraceID.h"
23-
#include "jaegertracing/Tracer.h"
2424
#include "jaegertracing/baggage/RestrictionsConfig.h"
2525
#include "jaegertracing/net/IPAddress.h"
2626
#include "jaegertracing/propagation/HeadersConfig.h"
@@ -175,13 +175,13 @@ TEST(Tracer, testTracer)
175175

176176
span->SetOperationName("test-set-operation");
177177
span->SetTag("tag-key", "tag-value");
178-
span->SetBaggageItem("test-baggage-item-key", "test-baggage-item-value");
179-
ASSERT_EQ("test-baggage-item-value",
178+
span->SetBaggageItem("test-baggage-item-key", "test baggage item value");
179+
ASSERT_EQ("test baggage item value",
180180
span->BaggageItem("test-baggage-item-key"));
181181
span->Log({ { "log-bool", true } });
182182
opentracing::FinishSpanOptions foptions;
183183
opentracing::LogRecord lr{};
184-
lr.fields = { {"options-log", "yep"} };
184+
lr.fields = { { "options-log", "yep" } };
185185
foptions.log_records.push_back(std::move(lr));
186186
lr.timestamp = opentracing::SystemClock::now();
187187
span->FinishWithOptions(foptions);
@@ -277,7 +277,7 @@ TEST(Tracer, testPropagation)
277277
std::static_pointer_cast<Tracer>(opentracing::Tracer::Global());
278278
const std::unique_ptr<Span> span(static_cast<Span*>(
279279
tracer->StartSpanWithOptions("test-inject", {}).release()));
280-
span->SetBaggageItem("test-baggage-item-key", "test-baggage-item-value");
280+
span->SetBaggageItem("test-baggage-item-key", "test baggage item value");
281281

282282
// Binary
283283
{
@@ -305,7 +305,7 @@ TEST(Tracer, testPropagation)
305305
std::ostringstream oss;
306306
oss << span->context();
307307
ASSERT_EQ(oss.str(), textMap.at(kTraceContextHeaderName));
308-
ASSERT_EQ("test-baggage-item-value",
308+
ASSERT_EQ("test baggage item value",
309309
textMap.at(std::string(kTraceBaggageHeaderPrefix) +
310310
"test-baggage-item-key"));
311311
ReaderMock<opentracing::TextMapReader> textReader(textMap);
@@ -327,7 +327,7 @@ TEST(Tracer, testPropagation)
327327
std::ostringstream oss;
328328
oss << span->context();
329329
ASSERT_EQ(oss.str(), headerMap.at(kTraceContextHeaderName));
330-
ASSERT_EQ("test-baggage-item-value",
330+
ASSERT_EQ("test baggage item value",
331331
headerMap.at(std::string(kTraceBaggageHeaderPrefix) +
332332
"test-baggage-item-key"));
333333
ReaderMock<opentracing::HTTPHeadersReader> headerReader(headerMap);
@@ -337,6 +337,65 @@ TEST(Tracer, testPropagation)
337337
static_cast<SpanContext*>(result->release()));
338338
ASSERT_TRUE(static_cast<bool>(extractedCtx));
339339
ASSERT_EQ(span->context(), *extractedCtx);
340+
341+
// Test debug header.
342+
headerMap.clear();
343+
headerMap[kJaegerDebugHeader] = "yes";
344+
tracer->Inject(span->context(), headerWriter);
345+
result = tracer->Extract(headerReader);
346+
ASSERT_TRUE(static_cast<bool>(result));
347+
extractedCtx.reset(static_cast<SpanContext*>(result->release()));
348+
ASSERT_TRUE(static_cast<bool>(extractedCtx));
349+
ASSERT_NE(span->context(), *extractedCtx);
350+
SpanContext ctx(
351+
span->context().traceID(),
352+
span->context().spanID(),
353+
span->context().parentID(),
354+
span->context().flags() |
355+
static_cast<unsigned char>(SpanContext::Flag::kDebug),
356+
span->context().baggage(),
357+
"yes");
358+
ASSERT_EQ(ctx, *extractedCtx);
359+
360+
// Test bad trace context.
361+
headerMap.clear();
362+
headerMap[kTraceContextHeaderName] = "12345678";
363+
result = tracer->Extract(headerReader);
364+
ASSERT_TRUE(static_cast<bool>(result));
365+
extractedCtx.reset(static_cast<SpanContext*>(result->release()));
366+
ASSERT_EQ(nullptr, extractedCtx.get());
367+
368+
// Test empty map.
369+
headerMap.clear();
370+
result = tracer->Extract(headerReader);
371+
ASSERT_TRUE(static_cast<bool>(result));
372+
extractedCtx.reset(static_cast<SpanContext*>(result->release()));
373+
ASSERT_EQ(nullptr, extractedCtx.get());
374+
375+
// Test alternative baggage format.
376+
headerMap.clear();
377+
ctx = SpanContext(span->context().traceID(),
378+
span->context().spanID(),
379+
span->context().parentID(),
380+
span->context().flags(),
381+
{ { "a", "x" }, { "b", "y" }, { "c", "z" } });
382+
tracer->Inject(ctx, headerWriter);
383+
for (auto itr = std::begin(headerMap); itr != std::end(headerMap);) {
384+
if (itr->first.substr(0, std::strlen(kTraceBaggageHeaderPrefix)) ==
385+
kTraceBaggageHeaderPrefix) {
386+
itr = headerMap.erase(itr);
387+
}
388+
else {
389+
++itr;
390+
}
391+
}
392+
headerMap[kJaegerBaggageHeader] = "a=x,b=y,c=z";
393+
result = tracer->Extract(headerReader);
394+
ASSERT_TRUE(static_cast<bool>(result));
395+
extractedCtx.reset(static_cast<SpanContext*>(result->release()));
396+
ASSERT_TRUE(static_cast<bool>(extractedCtx));
397+
ASSERT_EQ(3, extractedCtx->baggage().size());
398+
ASSERT_EQ(ctx, *extractedCtx);
340399
}
341400
}
342401

‎src/jaegertracing/propagation/Propagator.h‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class Propagator : public Extractor<ReaderType>, public Injector<WriterType> {
6969
if (key == _headerKeys.traceContextHeaderName()) {
7070
const auto safeValue = decodeValue(value);
7171
std::istringstream iss(safeValue);
72-
if (!(iss >> ctx)) {
72+
if (!(iss >> ctx) || ctx == SpanContext()) {
7373
return opentracing::make_expected_from_error<void>(
7474
opentracing::span_context_corrupted_error);
7575
}
@@ -104,10 +104,15 @@ class Propagator : public Extractor<ReaderType>, public Injector<WriterType> {
104104
return SpanContext();
105105
}
106106

107+
int flags = ctx.flags();
108+
if (!debugID.empty()) {
109+
flags |= static_cast<unsigned char>(SpanContext::Flag::kDebug) |
110+
static_cast<unsigned char>(SpanContext::Flag::kSampled);
111+
}
107112
return SpanContext(ctx.traceID(),
108113
ctx.spanID(),
109114
ctx.parentID(),
110-
ctx.flags(),
115+
flags,
111116
baggage,
112117
debugID);
113118
}
@@ -146,11 +151,9 @@ class Propagator : public Extractor<ReaderType>, public Injector<WriterType> {
146151
static StrMap parseCommaSeparatedMap(const std::string& escapedValue)
147152
{
148153
StrMap map;
149-
const auto value = net::URI::queryUnescape(escapedValue);
150-
for (auto pos = value.find(','), prev = static_cast<size_t>(0);
151-
pos != std::string::npos;
152-
prev = pos, pos = value.find(',', pos + 1)) {
153-
const auto piece = value.substr(prev, pos);
154+
std::istringstream iss(net::URI::queryUnescape(escapedValue));
155+
std::string piece;
156+
while (std::getline(iss, piece, ',')) {
154157
const auto eqPos = piece.find('=');
155158
if (eqPos != std::string::npos) {
156159
const auto key = piece.substr(0, eqPos);
@@ -212,6 +215,13 @@ class BinaryPropagator : public Extractor<std::istream&>,
212215
public:
213216
using StrMap = SpanContext::StrMap;
214217

218+
explicit BinaryPropagator(const std::shared_ptr<metrics::Metrics>& metrics =
219+
std::shared_ptr<metrics::Metrics>())
220+
: _metrics(metrics == nullptr ? metrics::Metrics::makeNullMetrics()
221+
: metrics)
222+
{
223+
}
224+
215225
void inject(const SpanContext& ctx, std::ostream& out) const override
216226
{
217227
writeBinary(out, ctx.traceID().high());
@@ -251,11 +261,17 @@ class BinaryPropagator : public Extractor<std::istream&>,
251261
for (auto i = static_cast<uint32_t>(0); i < numBaggageItems; ++i) {
252262
const auto keyLength = readBinary<uint32_t>(in);
253263
std::string key(keyLength, '\0');
254-
in.read(&key[0], keyLength);
264+
if (!in.read(&key[0], keyLength)) {
265+
_metrics->decodingErrors().inc(1);
266+
return SpanContext();
267+
}
255268

256269
const auto valueLength = readBinary<uint32_t>(in);
257270
std::string value(valueLength, '\0');
258-
in.read(&value[0], valueLength);
271+
if (!in.read(&value[0], valueLength)) {
272+
_metrics->decodingErrors().inc(1);
273+
return SpanContext();
274+
}
259275

260276
baggage[key] = value;
261277
}
@@ -294,6 +310,9 @@ class BinaryPropagator : public Extractor<std::istream&>,
294310
}
295311
return platform::endian::fromBigEndian(value);
296312
}
313+
314+
private:
315+
std::shared_ptr<metrics::Metrics> _metrics;
297316
};
298317

299318
} // namespace propagation

‎src/jaegertracing/propagation/PropagatorTest.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@
1414
* limitations under the License.
1515
*/
1616

17+
#include "jaegertracing/propagation/Propagator.h"
1718
#include "jaegertracing/SpanContext.h"
1819
#include "jaegertracing/TraceID.h"
19-
#include "jaegertracing/propagation/Propagator.h"
2020
#include <gtest/gtest.h>
2121
#include <iosfwd>
2222
#include <random>

0 commit comments

Comments
 (0)