Skip to content

Commit 03c0a00

Browse files
committed
refactor tracing: remove Tracer from Span API, separate link & parent_link
* remove most methods from `Tracer` as a part of efforts to deprecate and remove this API * remove `Tracer` from `Span` API * store `link` and `parent_link` separately * always fill in `link` at `Span::Impl` construction commit_hash:c2bd09579ba0e41d56f41651eca86908feb6f374
1 parent 9ee3736 commit 03c0a00

File tree

18 files changed

+185
-282
lines changed

18 files changed

+185
-282
lines changed

core/include/userver/rcu/fwd.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ namespace rcu {
1010
struct DefaultRcuTraits;
1111
struct SyncRcuTraits;
1212
struct BlockingRcuTraits;
13+
struct ExclusiveRcuTraits;
1314

1415
template <typename Key>
1516
struct DefaultRcuMapTraits;

core/include/userver/tracing/in_place_span.hpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,24 @@ namespace tracing {
1818
/// cause stack overflow.
1919
class InPlaceSpan final {
2020
public:
21+
struct DetachedTag {};
22+
2123
explicit InPlaceSpan(
2224
std::string&& name,
23-
utils::impl::SourceLocation source_location = utils::impl::SourceLocation::Current()
25+
const utils::impl::SourceLocation& source_location = utils::impl::SourceLocation::Current()
2426
);
2527

2628
explicit InPlaceSpan(
2729
std::string&& name,
2830
std::string&& trace_id,
2931
std::string&& parent_span_id,
30-
utils::impl::SourceLocation source_location = utils::impl::SourceLocation::Current()
32+
const utils::impl::SourceLocation& source_location = utils::impl::SourceLocation::Current()
33+
);
34+
35+
explicit InPlaceSpan(
36+
std::string&& name,
37+
DetachedTag,
38+
const utils::impl::SourceLocation& source_location = utils::impl::SourceLocation::Current()
3139
);
3240

3341
InPlaceSpan(InPlaceSpan&&) = delete;
@@ -43,7 +51,7 @@ class InPlaceSpan final {
4351

4452
private:
4553
struct Impl;
46-
utils::FastPimpl<Impl, 4240, 8> impl_;
54+
utils::FastPimpl<Impl, 4288, 8> impl_;
4755
};
4856

4957
} // namespace tracing

core/include/userver/tracing/span.hpp

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,6 @@ class Span final {
3434
public:
3535
class Impl;
3636

37-
explicit Span(
38-
TracerPtr tracer,
39-
std::string name,
40-
const Span* parent,
41-
ReferenceType reference_type,
42-
logging::Level log_level = logging::Level::kInfo,
43-
utils::impl::SourceLocation source_location = utils::impl::SourceLocation::Current()
44-
);
45-
4637
/// Use default tracer and implicit coro local storage for parent
4738
/// identification, takes TraceID from the parent.
4839
///
@@ -52,21 +43,22 @@ class Span final {
5243
std::string name,
5344
ReferenceType reference_type = ReferenceType::kChild,
5445
logging::Level log_level = logging::Level::kInfo,
55-
utils::impl::SourceLocation source_location = utils::impl::SourceLocation::Current()
46+
const utils::impl::SourceLocation& source_location = utils::impl::SourceLocation::Current()
5647
);
5748

58-
/// @cond
59-
// For internal use only
60-
explicit Span(Span::Impl& impl);
61-
/// @endcond
49+
explicit Span(
50+
std::string name,
51+
const Span* parent,
52+
ReferenceType reference_type = ReferenceType::kChild,
53+
logging::Level log_level = logging::Level::kInfo,
54+
const utils::impl::SourceLocation& source_location = utils::impl::SourceLocation::Current()
55+
);
6256

6357
Span(Span&& other) noexcept;
64-
65-
~Span();
66-
67-
Span& operator=(const Span&) = delete;
68-
58+
Span(const Span& other) = delete;
6959
Span& operator=(Span&&) = delete;
60+
Span& operator=(const Span&) = delete;
61+
~Span();
7062

7163
/// @brief Returns the Span of the current task.
7264
///
@@ -214,11 +206,9 @@ class Span final {
214206
///
215207
/// Propagates within a single service, but not from client to server. A new
216208
/// link is generated for the "root" request handling task
209+
// TODO restrict SetLink to SpanBuilder.
217210
void SetLink(std::string link);
218211

219-
/// Set parent_link - an ID . Can be called only once.
220-
void SetParentLink(std::string parent_link);
221-
222212
/// Get link - a request ID within the service.
223213
///
224214
/// Propagates within a single service, but not from client to server. A new
@@ -287,6 +277,7 @@ class Span final {
287277

288278
friend class SpanBuilder;
289279
friend class TagScope;
280+
friend class InPlaceSpan;
290281

291282
explicit Span(std::unique_ptr<Impl, OptionalDeleter>&& pimpl);
292283

core/include/userver/tracing/span_builder.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
/// @file userver/tracing/span_builder.hpp
44
/// @brief @copybrief tracing::SpanBuilder
55

6+
#include <cstddef>
67
#include <string>
78

89
#include <userver/tracing/span.hpp>
@@ -23,11 +24,14 @@ class SpanBuilder final {
2324
void SetTraceId(std::string trace_id);
2425
std::string_view GetTraceId() const noexcept;
2526
void SetSpanId(std::string span_id);
27+
void SetLink(std::string link);
2628
void SetParentSpanId(std::string parent_span_id);
2729
void SetParentLink(std::string parent_link);
2830
void AddTagFrozen(std::string key, logging::LogExtra::Value value);
2931
void AddNonInheritableTag(std::string key, logging::LogExtra::Value value);
32+
3033
Span Build() &&;
34+
Span BuildDetachedFromCoroStack() &&;
3135

3236
private:
3337
std::unique_ptr<Span::Impl, Span::OptionalDeleter> pimpl_;

core/include/userver/tracing/tracer.hpp

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,53 +2,59 @@
22

33
#include <memory>
44

5+
#include <userver/rcu/fwd.hpp>
56
#include <userver/tracing/span.hpp>
67
#include <userver/tracing/tracer_fwd.hpp>
78

89
#include <dynamic_config/variables/USERVER_NO_LOG_SPANS.hpp>
910

1011
USERVER_NAMESPACE_BEGIN
1112

12-
namespace logging::impl {
13-
class TagWriter;
14-
} // namespace logging::impl
15-
1613
/// Opentracing support
1714
namespace tracing {
1815

1916
using NoLogSpans = ::dynamic_config::userver_no_log_spans::VariableType;
2017

21-
class Tracer : public std::enable_shared_from_this<Tracer> {
18+
void SetNoLogSpans(NoLogSpans&& spans);
19+
bool IsNoLogSpan(const std::string& name);
20+
NoLogSpans CopyNoLogSpans();
21+
22+
// For legacy opentracing support only.
23+
class Tracer final {
2224
public:
23-
static void SetNoLogSpans(NoLogSpans&& spans);
24-
static bool IsNoLogSpan(const std::string& name);
25+
static void SetTracer(Tracer&& tracer);
2526

26-
static void SetTracer(TracerPtr tracer);
27+
static rcu::ReadablePtr<Tracer, rcu::ExclusiveRcuTraits> ReadTracer();
2728

28-
static TracerPtr GetTracer();
29+
static Tracer CopyCurrentTracer();
2930

30-
const std::string& GetServiceName() const;
31+
Tracer(std::string_view service_name, logging::LoggerPtr optional_logger)
32+
: service_name_(service_name), optional_logger_(std::move(optional_logger)) {}
3133

32-
Span CreateSpanWithoutParent(std::string name);
34+
// Only works if legacy opentracing is set up.
35+
const std::string& GetServiceName() const;
3336

34-
Span CreateSpan(std::string name, const Span& parent, ReferenceType reference_type);
37+
const logging::LoggerPtr& GetOptionalLogger() const { return optional_logger_; }
3538

36-
logging::LoggerPtr GetOptionalLogger() const { return optional_logger_; }
39+
private:
40+
std::string service_name_;
41+
logging::LoggerPtr optional_logger_;
42+
};
3743

38-
protected:
39-
explicit Tracer(std::string_view service_name, logging::LoggerPtr optional_logger)
40-
: service_name_(service_name), optional_logger_(std::move(optional_logger)) {}
44+
// For tests and benchmarks only!
45+
class TracerCleanupScope final {
46+
public:
47+
TracerCleanupScope();
4148

42-
virtual ~Tracer();
49+
TracerCleanupScope(TracerCleanupScope&&) = delete;
50+
TracerCleanupScope& operator=(TracerCleanupScope&&) = delete;
51+
~TracerCleanupScope();
4352

4453
private:
45-
const std::string service_name_;
46-
const logging::LoggerPtr optional_logger_;
54+
Tracer old_tracer_;
55+
NoLogSpans old_no_log_spans_;
4756
};
4857

49-
/// Make a tracer that could be set globally via tracing::Tracer::SetTracer
50-
TracerPtr MakeTracer(std::string_view service_name, logging::LoggerPtr logger, std::string_view tracer_type = "native");
51-
5258
} // namespace tracing
5359

5460
USERVER_NAMESPACE_END

core/include/userver/utils/async.hpp

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ namespace impl {
2222
struct SpanWrapCall {
2323
enum class InheritVariables { kYes, kNo };
2424

25-
explicit SpanWrapCall(std::string&& name, InheritVariables inherit_variables);
25+
explicit SpanWrapCall(std::string&& name, InheritVariables inherit_variables, const SourceLocation& location);
2626

2727
SpanWrapCall(const SpanWrapCall&) = delete;
2828
SpanWrapCall(SpanWrapCall&&) = delete;
@@ -41,14 +41,20 @@ struct SpanWrapCall {
4141

4242
struct Impl;
4343

44-
static constexpr std::size_t kImplSize = 4280;
44+
static constexpr std::size_t kImplSize = 4328;
4545
static constexpr std::size_t kImplAlign = 8;
4646
utils::FastPimpl<Impl, kImplSize, kImplAlign> pimpl_;
4747
};
4848

49-
// Note: 'name' must outlive the result of this function
50-
inline auto SpanLazyPrvalue(std::string&& name) {
51-
return utils::LazyPrvalue([&name] { return SpanWrapCall(std::move(name), SpanWrapCall::InheritVariables::kYes); });
49+
// Note: 'name' and 'location' must outlive the result of this function
50+
inline auto SpanLazyPrvalue(
51+
std::string&& name,
52+
SpanWrapCall::InheritVariables inherit_variables = SpanWrapCall::InheritVariables::kYes,
53+
const SourceLocation& location = SourceLocation::Current()
54+
) {
55+
return utils::LazyPrvalue([&name, inherit_variables, &location] {
56+
return SpanWrapCall(std::move(name), inherit_variables, location);
57+
});
5258
}
5359

5460
} // namespace impl
@@ -366,9 +372,7 @@ template <typename Function, typename... Args>
366372
AsyncBackground(std::string name, engine::TaskProcessor& task_processor, Function&& f, Args&&... args) {
367373
return engine::AsyncNoSpan(
368374
task_processor,
369-
utils::LazyPrvalue([&] {
370-
return impl::SpanWrapCall(std::move(name), impl::SpanWrapCall::InheritVariables::kNo);
371-
}),
375+
impl::SpanLazyPrvalue(std::move(name), impl::SpanWrapCall::InheritVariables::kNo),
372376
std::forward<Function>(f),
373377
std::forward<Args>(args)...
374378
);
@@ -392,9 +396,7 @@ template <typename Function, typename... Args>
392396
CriticalAsyncBackground(std::string name, engine::TaskProcessor& task_processor, Function&& f, Args&&... args) {
393397
return engine::CriticalAsyncNoSpan(
394398
task_processor,
395-
utils::LazyPrvalue([&] {
396-
return impl::SpanWrapCall(std::move(name), impl::SpanWrapCall::InheritVariables::kNo);
397-
}),
399+
impl::SpanLazyPrvalue(std::move(name), impl::SpanWrapCall::InheritVariables::kNo),
398400
std::forward<Function>(f),
399401
std::forward<Args>(args)...
400402
);

core/src/components/component_list_test.hpp

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,26 +49,13 @@ inline constexpr std::string_view kMinimalStaticConfig = R"(
4949
format: ltsv
5050
)";
5151

52-
struct TracingGuard final {
53-
TracingGuard() : tracer(tracing::Tracer::GetTracer()) {}
54-
55-
~TracingGuard() {
56-
if (tracing::Tracer::GetTracer() != tracer) {
57-
tracing::Tracer::SetTracer(tracer);
58-
}
59-
}
60-
61-
const logging::LoggerPtr opentracing_logger;
62-
const tracing::TracerPtr tracer;
63-
};
64-
6552
std::string MergeYaml(std::string_view source, std::string_view patch);
6653

6754
} // namespace tests
6855

6956
class ComponentList : public ::testing::Test {
7057
tests::impl::DefaultLoggerGuardTest default_logger_guard_;
71-
tests::TracingGuard tracing_guard_;
58+
tracing::TracerCleanupScope tracer_scope_;
7259
};
7360

7461
USERVER_NAMESPACE_END

core/src/components/logging_configurator.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ LoggingConfigurator::~LoggingConfigurator() { config_subscription_.Unsubscribe()
4343

4444
void LoggingConfigurator::OnConfigUpdate(const dynamic_config::Snapshot& config) {
4545
(void)this; // silence clang-tidy
46-
tracing::Tracer::SetNoLogSpans(tracing::NoLogSpans{config[::dynamic_config::USERVER_NO_LOG_SPANS]});
46+
tracing::SetNoLogSpans(tracing::NoLogSpans{config[::dynamic_config::USERVER_NO_LOG_SPANS]});
4747

4848
try {
4949
const auto& dd = config[kDynamicDebugConfig];

core/src/tracing/component.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@ Tracer::Tracer(const ComponentConfig& config, const ComponentContext& context) {
3232
LOG_INFO() << "Opentracing logger is not registered";
3333
}
3434

35-
tracing::Tracer::SetTracer(
36-
tracing::MakeTracer(std::move(service_name), std::move(opentracing_logger), tracer_type)
37-
);
35+
tracing::Tracer::SetTracer(tracing::Tracer(std::move(service_name), std::move(opentracing_logger)));
3836
} else {
3937
throw std::runtime_error("Tracer type is not supported: " + tracer_type);
4038
}

core/src/tracing/in_place_span.cpp

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,55 +3,47 @@
33
#include <utility>
44

55
#include <tracing/span_impl.hpp>
6-
#include <userver/utils/uuid4.hpp>
76

87
USERVER_NAMESPACE_BEGIN
98

109
namespace tracing {
1110

12-
namespace {
13-
14-
void SetLinkIfRoot(tracing::Span& span) {
15-
if (span.GetLink().empty()) {
16-
span.SetLink(utils::generators::GenerateUuid());
17-
}
18-
}
19-
20-
} // namespace
21-
2211
struct InPlaceSpan::Impl final {
2312
template <typename... Args>
24-
explicit Impl(Args&&... args) : span_impl(std::forward<Args>(args)...), span(span_impl) {}
13+
explicit Impl(Args&&... args)
14+
: span_impl(std::forward<Args>(args)...),
15+
span(std::unique_ptr<Span::Impl, Span::OptionalDeleter>(&span_impl, Span::OptionalDeleter::DoNotDelete())) {}
2516

26-
tracing::Span::Impl span_impl;
27-
tracing::Span span;
17+
Span::Impl span_impl;
18+
Span span;
2819
};
2920

30-
InPlaceSpan::InPlaceSpan(std::string&& name, utils::impl::SourceLocation source_location)
31-
: impl_(std::move(name), ReferenceType::kChild, logging::Level::kInfo, std::move(source_location)) {
21+
InPlaceSpan::InPlaceSpan(std::string&& name, const utils::impl::SourceLocation& source_location)
22+
: impl_(std::move(name), GetParentSpanImpl(), ReferenceType::kChild, logging::Level::kInfo, source_location) {
3223
impl_->span.AttachToCoroStack();
33-
SetLinkIfRoot(impl_->span);
3424
}
3525

3626
InPlaceSpan::InPlaceSpan(
3727
std::string&& name,
3828
std::string&& trace_id,
3929
std::string&& parent_span_id,
40-
utils::impl::SourceLocation source_location
30+
const utils::impl::SourceLocation& source_location
4131
)
42-
: impl_(std::move(name), ReferenceType::kChild, logging::Level::kInfo, std::move(source_location)) {
32+
: impl_(std::move(name), GetParentSpanImpl(), ReferenceType::kChild, logging::Level::kInfo, source_location) {
4333
impl_->span.AttachToCoroStack();
4434
impl_->span_impl.SetTraceId(std::move(trace_id));
4535
impl_->span_impl.SetParentId(std::move(parent_span_id));
46-
SetLinkIfRoot(impl_->span);
4736
}
4837

38+
InPlaceSpan::InPlaceSpan(std::string&& name, DetachedTag, const utils::impl::SourceLocation& source_location)
39+
: impl_(std::move(name), GetParentSpanImpl(), ReferenceType::kChild, logging::Level::kInfo, source_location) {}
40+
4941
InPlaceSpan::~InPlaceSpan() = default;
5042

5143
tracing::Span& InPlaceSpan::Get() noexcept { return impl_->span; }
5244

5345
void InPlaceSpan::SetParentLink(utils::impl::InternalTag, std::string&& parent_link) {
54-
impl_->span.SetParentLink(std::move(parent_link));
46+
impl_->span_impl.SetParentLink(std::move(parent_link));
5547
}
5648

5749
} // namespace tracing

0 commit comments

Comments
 (0)