Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion source/extensions/tracers/datadog/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ envoy_cc_library(
srcs = [
"datadog_tracer_impl.cc",
"dict_util.cc",
"span.cc",
"time_util.cc",
],
hdrs = [
"datadog_tracer_impl.h",
"dict_util.h",
"span.h",
"time_util.h",
"tracer_stats.h",
],
Expand All @@ -30,8 +32,8 @@ envoy_cc_library(
"-DDD_USE_ABSEIL_FOR_ENVOY",
],
external_deps = [
"dd_opentracing_cpp",
"dd_trace_cpp",
"dd_opentracing_cpp",
],
deps = [
"//source/common/config:utility_lib",
Expand Down
3 changes: 1 addition & 2 deletions source/extensions/tracers/datadog/datadog_tracer_impl.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
#pragma once

#include <datadog/opentracing.h>

#include "envoy/config/trace/v3/datadog.pb.h"
#include "envoy/local_info/local_info.h"
#include "envoy/runtime/runtime.h"
Expand All @@ -14,6 +12,7 @@
#include "source/common/upstream/cluster_update_tracker.h"
#include "source/extensions/tracers/common/ot/opentracing_driver_impl.h"

#include "datadog/opentracing.h"
#include "fmt/ostream.h"

namespace Envoy {
Expand Down
6 changes: 3 additions & 3 deletions source/extensions/tracers/datadog/dict_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
* headers, or writing HTTP request headers.
*/

#include <datadog/dict_reader.h>
#include <datadog/dict_writer.h>

#include <string>

#include "datadog/dict_reader.h"
#include "datadog/dict_writer.h"

namespace Envoy {
namespace Tracing {
class TraceContext;
Expand Down
117 changes: 117 additions & 0 deletions source/extensions/tracers/datadog/span.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
#include "source/extensions/tracers/datadog/span.h"

#include <utility>

#include "source/common/tracing/null_span_impl.h"
#include "source/extensions/tracers/datadog/time_util.h"

#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
#include "datadog/dict_writer.h"
#include "datadog/sampling_priority.h"
#include "datadog/span_config.h"
#include "datadog/trace_segment.h"

namespace Envoy {
namespace Extensions {
namespace Tracers {
namespace Datadog {
namespace {

class TraceContextWriter : public datadog::tracing::DictWriter {
public:
explicit TraceContextWriter(Tracing::TraceContext& context) : context_(context) {}

void set(datadog::tracing::StringView key, datadog::tracing::StringView value) override {
context_.setByKey(key, value);
}

private:
Tracing::TraceContext& context_;
};

} // namespace

Span::Span(datadog::tracing::Span&& span) : span_(std::move(span)) {}

const datadog::tracing::Optional<datadog::tracing::Span>& Span::impl() const { return span_; }

void Span::setOperation(absl::string_view operation) {
if (!span_) {
return;
}

span_->set_name(operation);
}

void Span::setTag(absl::string_view name, absl::string_view value) {
if (!span_) {
return;
}

span_->set_tag(name, value);
}

void Span::log(SystemTime, const std::string&) {
// Datadog spans don't have in-bound "events" or "logs".
}

void Span::finishSpan() { span_.reset(); }
Comment thread
wbpcode marked this conversation as resolved.

void Span::injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr&) {
if (!span_) {
return;
}

TraceContextWriter writer{trace_context};
span_->inject(writer);
}

Tracing::SpanPtr Span::spawnChild(const Tracing::Config&, const std::string& name,
SystemTime start_time) {
if (!span_) {
// I don't expect this to happen. This means that `spawnChild` was called
// after `finishSpan`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we use an ENVOY_BUG here (logs a message in production build, asserts in debug/default builds)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That might be worth doing.

I'd reserve assert and friends for cases where the detected behavior is in violation of the component's contract. Because this Span has the escape hatch of going into "null-mode," it's not necessary. By that reasoning, perhaps the code comment is not necessary either.

You mentioned the idea of @moderation looking over the behavior of this Span. Maybe they can decide whether "spawn-after-finish" ought to be considered forbidden in these tracing implementations.

return std::make_unique<Tracing::NullSpan>();
}

// The OpenTracing implementation ignored the `Tracing::Config` argument,
// so we will as well.
datadog::tracing::SpanConfig config;
config.name = name;
config.start = estimateTime(start_time);

return std::make_unique<Span>(span_->create_child(config));
}

void Span::setSampled(bool sampled) {
if (!span_) {
return;
}

auto priority = static_cast<int>(sampled ? datadog::tracing::SamplingPriority::USER_KEEP
: datadog::tracing::SamplingPriority::USER_DROP);
span_->trace_segment().override_sampling_priority(priority);
}

std::string Span::getBaggage(absl::string_view) {
// not implemented
return std::string{};
}

void Span::setBaggage(absl::string_view, absl::string_view) {
// not implemented
}

std::string Span::getTraceIdAsHex() const {
if (!span_) {
return std::string{};
}
return absl::StrCat(absl::Hex(span_->id()));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@dgoffredo Is this right?

}

} // namespace Datadog
} // namespace Tracers
} // namespace Extensions
} // namespace Envoy
59 changes: 59 additions & 0 deletions source/extensions/tracers/datadog/span.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
#pragma once

#include <optional>

#include "envoy/common/time.h"
#include "envoy/tracing/trace_driver.h"

#include "datadog/span.h"

namespace Envoy {
namespace Extensions {
namespace Tracers {
namespace Datadog {

/**
* Tracing::Span implementation for use in Datadog tracing. This class contains
* an optional<datadog::tracing::Span> and forwards its member functions to the
* corresponding member functions of datadog::tracing::Span.
*
* datadog::tracing::Span is the span type used in Datadog's core tracing
* library, dd-trace-cpp. It's wrapped in an optional<> because the lifetime
* of a datadog::tracing::Span is tied to the scope of the object itself,
* whereas the Tracing::Span implemented here has a finishSpan() member
* function that allows the span's lifetime to end without destroying the
* object.
*
* For the same reason, this class has two states: one when the
* optional<datadog::tracing::Span> is not empty and member functions are
* forwarded to it, and another state when the optional<datadog::tracing::Span>
* is empty and member functions have no effect.
*/
class Span : public Tracing::Span {
public:
explicit Span(datadog::tracing::Span&& span);

const datadog::tracing::Optional<datadog::tracing::Span>& impl() const;

Comment thread
dgoffredo marked this conversation as resolved.
// Envoy::Tracing::Span
void setOperation(absl::string_view operation) override;
void setTag(absl::string_view name, absl::string_view value) override;
void log(SystemTime, const std::string&) override;
void finishSpan() override;
void injectContext(Tracing::TraceContext& trace_context,
const Upstream::HostDescriptionConstSharedPtr& upstream) override;
Tracing::SpanPtr spawnChild(const Tracing::Config& config, const std::string& name,
SystemTime start_time) override;
void setSampled(bool sampled) override;
std::string getBaggage(absl::string_view key) override;
void setBaggage(absl::string_view key, absl::string_view value) override;
std::string getTraceIdAsHex() const override;

private:
datadog::tracing::Optional<datadog::tracing::Span> span_;
};

} // namespace Datadog
} // namespace Tracers
} // namespace Extensions
} // namespace Envoy
12 changes: 6 additions & 6 deletions source/extensions/tracers/datadog/time_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
/**
* This file contains functions related to time points and durations.
*
* Envoy has a time type that contains both a system time point and a steady
* ("monotonic") time point. However, only the system time is exposed to the
* tracing subsystem. This may be remedied in the future, but for now we work
* with the system time.
* Envoy has a TimeSource abstraction that provides both a system time point and
* a steady ("monotonic") time point. However, only the system time is exposed
* to the tracing subsystem. This may be remedied in the future, but for now we
* work with the system time.
*
* This is problematic for the Datadog core tracing library (dd-trace-cpp),
* because it uses the steady time to calculate the duration of a span
Expand All @@ -19,10 +19,10 @@
* estimateTime function.
*/

#include <datadog/clock.h>

#include "envoy/common/time.h"

#include "datadog/clock.h"

namespace Envoy {
namespace Extensions {
namespace Tracers {
Expand Down
1 change: 1 addition & 0 deletions test/extensions/tracers/datadog/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ envoy_extension_cc_test(
srcs = [
"datadog_tracer_impl_test.cc",
"dict_util_test.cc",
"span_test.cc",
"time_util_test.cc",
"tracer_stats_test.cc",
],
Expand Down
3 changes: 1 addition & 2 deletions test/extensions/tracers/datadog/dict_util_test.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#include <datadog/optional.h>

#include <utility>
#include <vector>

Expand All @@ -10,6 +8,7 @@
#include "test/mocks/http/mocks.h"
#include "test/test_common/utility.h"

#include "datadog/optional.h"
#include "gtest/gtest.h"

namespace Envoy {
Expand Down
Loading