Skip to content
Merged
6 changes: 6 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ set(CMAKE_CXX_EXTENSIONS OFF)
# <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108088>.
add_compile_options(-Wno-error=free-nonheap-object)

# This warning has a false positive with clang. See
# <https://stackoverflow.com/questions/52416362>.
if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
add_compile_options(-Wno-error=unused-lambda-capture)
endif()

# If we're building with clang, then use the libc++ version of the standard
# library instead of libstdc++. Better coverage of build configurations.
#
Expand Down
11 changes: 9 additions & 2 deletions src/datadog/id_generator.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "id_generator.h"

#include <bitset>
#include <chrono>

#include "random.h"

Expand All @@ -15,11 +16,17 @@ class DefaultIDGenerator : public IDGenerator {
explicit DefaultIDGenerator(bool trace_id_128_bit)
: trace_id_128_bit_(trace_id_128_bit) {}

TraceID trace_id() const override {
TraceID trace_id(const TimePoint& start) const override {
TraceID result;
result.low = random_uint64();
if (trace_id_128_bit_) {
result.high = random_uint64();
// Highest 32 bits contain a unix timestamp (the trace start time).
const auto since_epoch = start.wall.time_since_epoch();
const auto seconds =
std::chrono::duration_cast<std::chrono::seconds>(since_epoch).count();
// The farthest we'll go back is the unix epoch.
const std::uint64_t unsigned_seconds = seconds < 0 ? 0 : seconds;
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.

It's not actually going to happen, but the lowest value should probably be 1, and not zero.

Copy link
Copy Markdown
Contributor Author

@dgoffredo dgoffredo Jul 7, 2023

Choose a reason for hiding this comment

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

I agree it's not going to happen. But why 1 and not 0?

My understanding of unix time is that it is the number of seconds elapsed since 00:00:00 UTC on 1 January 1970.

Hm, wait...

david@ein:~/Downloads$ python
Python 3.10.6 (main, May 29 2023, 11:10:38) [GCC 11.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import datetime     
>>> datetime.datetime.fromtimestamp(0)
datetime.datetime(1969, 12, 31, 19, 0)

Ah, that's a time zone thing.

Anyway, the reason I have a cutoff here is to prevent the rollover to 18446744073709551615 at -1 (which is not a valid unix timestamp, but would correspond to a date over 584 trillion years in the future -- build software to last!).

result.high = unsigned_seconds << 32;
} else {
// In 64-bit mode, zero the most significant bit for compatibility with
// older tracers that can't accept values above
Expand Down
5 changes: 4 additions & 1 deletion src/datadog/id_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <cstdint>
#include <memory>

#include "clock.h"
#include "trace_id.h"

namespace datadog {
Expand All @@ -21,8 +22,10 @@ class IDGenerator {
public:
virtual ~IDGenerator() = default;

// Generate a span ID.
virtual std::uint64_t span_id() const = 0;
virtual TraceID trace_id() const = 0;
// Generate a trace ID for a trace having the specified `start` time.
virtual TraceID trace_id(const TimePoint& start) const = 0;
};

std::shared_ptr<const IDGenerator> default_id_generator(bool trace_id_128_bit);
Expand Down
2 changes: 0 additions & 2 deletions src/datadog/tags.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ const std::string service_name = "service.name";
const std::string span_type = "span.type";
const std::string operation_name = "operation";
const std::string resource_name = "resource.name";
const std::string manual_keep = "manual.keep";
const std::string manual_drop = "manual.drop";
const std::string version = "version";

namespace internal {
Expand Down
2 changes: 0 additions & 2 deletions src/datadog/tags.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ extern const std::string service_name;
extern const std::string span_type;
extern const std::string operation_name;
extern const std::string resource_name;
extern const std::string manual_keep;
extern const std::string manual_drop;
extern const std::string version;

namespace internal {
Expand Down
35 changes: 31 additions & 4 deletions src/datadog/tracer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ void handle_trace_tags(StringView trace_tags, ExtractedData& result,
error->with_prefix("Unable to parse high bits of the trace ID in "
"Datadog style from the "
"\"_dd.p.tid\" trace tag: "));
span_tags[tags::internal::propagation_error] = "decoding_error";
span_tags[tags::internal::propagation_error] = "malformed_tid " + value;
continue;
}
// Note that this assumes the lower 64 bits of the trace ID have already
// been extracted (i.e. we look for X-Datadog-Trace-ID first).

if (result.trace_id) {
// Note that this assumes the lower 64 bits of the trace ID have already
// been extracted (i.e. we look for X-Datadog-Trace-ID first).
result.trace_id->high = *high;
}
}
Expand Down Expand Up @@ -269,7 +271,7 @@ Span Tracer::create_span(const SpanConfig& config) {
auto span_data = std::make_unique<SpanData>();
span_data->apply_config(*defaults_, config, clock_);
std::vector<std::pair<std::string, std::string>> trace_tags;
span_data->trace_id = generator_->trace_id();
span_data->trace_id = generator_->trace_id(span_data->start);
if (span_data->trace_id.high) {
trace_tags.emplace_back(tags::internal::trace_id_high,
hex(span_data->trace_id.high));
Expand Down Expand Up @@ -392,6 +394,31 @@ Expected<Span> Tracer::extract_span(const DictReader& reader,
span_data->trace_id = *trace_id;
span_data->parent_id = *parent_id;

if (span_data->trace_id.high) {
// The trace ID has some bits set in the higher 64 bits. Set the
// corresponding `trace_id_high` tag, so that the Datadog backend is aware
// of those bits.
//
// First, though, if the `trace_id_high` tag is already set and has a bogus
// value or a value inconsistent with the trace ID, tag an error.
const std::string hex_high = hex(span_data->trace_id.high);
const auto extant = std::find_if(
trace_tags.begin(), trace_tags.end(), [&](const auto& pair) {
return pair.first == tags::internal::trace_id_high;
});
if (extant == trace_tags.end()) {
trace_tags.emplace_back(tags::internal::trace_id_high, hex_high);
} else if (!parse_uint64(extant->second, 16)) {
span_data->tags[tags::internal::propagation_error] =
"malformed_tid " + extant->second;
extant->second = hex_high;
} else if (extant->second != hex_high) {
span_data->tags[tags::internal::propagation_error] =
"inconsistent_tid " + extant->second;
extant->second = hex_high;
}
}

Optional<SamplingDecision> sampling_decision;
if (sampling_priority) {
SamplingDecision decision;
Expand Down
6 changes: 0 additions & 6 deletions src/datadog/w3c_propagation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,12 +214,6 @@ void parse_datadog_tracestate(ExtractedData& result, StringView datadog_value) {
const auto tag_suffix = key.substr(2);
std::string tag_name = "_dd.p.";
append(tag_name, tag_suffix);
// The "_dd.p.tid" trace tag is ignored in this context, since its value
// is better inferred from the higher 64 bits of the trace ID.
if (tag_name == "_dd.p.tid") {
pair_begin = pair_end == end ? end : pair_end + 1;
continue;
}
// The tag value was encoded with all '=' replaced by '~'. Undo that
// transformation.
std::string decoded_value{value};
Expand Down
8 changes: 4 additions & 4 deletions test/test_span.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ TEST_CASE("injection") {
struct Generator : public IDGenerator {
const std::uint64_t id;
explicit Generator(std::uint64_t id) : id(id) {}
TraceID trace_id() const override { return TraceID(id); }
TraceID trace_id(const TimePoint&) const override { return TraceID(id); }
std::uint64_t span_id() const override { return id; }
};
Tracer tracer{*finalized_config, std::make_shared<Generator>(42)};
Expand Down Expand Up @@ -478,7 +478,7 @@ TEST_CASE("injecting W3C traceparent header") {
struct Generator : public IDGenerator {
const std::uint64_t id;
explicit Generator(std::uint64_t id) : id(id) {}
TraceID trace_id() const override { return TraceID(id); }
TraceID trace_id(const TimePoint&) const override { return TraceID(id); }
std::uint64_t span_id() const override { return id; }
};
Tracer tracer{*finalized_config,
Expand Down Expand Up @@ -515,7 +515,7 @@ TEST_CASE("injecting W3C traceparent header") {
struct Generator : public IDGenerator {
const std::uint64_t id;
explicit Generator(std::uint64_t id) : id(id) {}
TraceID trace_id() const override { return TraceID(id); }
TraceID trace_id(const TimePoint&) const override { return TraceID(id); }
std::uint64_t span_id() const override { return id; }
};
Tracer tracer{*finalized_config, std::make_shared<Generator>(expected_id)};
Expand Down Expand Up @@ -715,7 +715,7 @@ TEST_CASE("128-bit trace ID injection") {

public:
explicit MockIDGenerator(TraceID trace_id) : trace_id_(trace_id) {}
TraceID trace_id() const override { return trace_id_; }
TraceID trace_id(const TimePoint&) const override { return trace_id_; }
// `span_id` won't be called, because root spans use the lower part of
// `trace_id` for the span ID.
std::uint64_t span_id() const override { return 42; }
Expand Down
Loading