Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set span start and end timestamps #98

Merged
merged 2 commits into from
Jun 9, 2020
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
10 changes: 10 additions & 0 deletions api/include/opentelemetry/core/timestamp.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ class SystemTimestamp
return nanos_since_epoch_ == other.nanos_since_epoch_;
}

bool operator!=(const SystemTimestamp &other) const noexcept
{
return nanos_since_epoch_ != other.nanos_since_epoch_;
}

private:
int64_t nanos_since_epoch_;
};
Expand Down Expand Up @@ -89,6 +94,11 @@ class SteadyTimestamp
return nanos_since_epoch_ == other.nanos_since_epoch_;
}

bool operator!=(const SteadyTimestamp &other) const noexcept
{
return nanos_since_epoch_ != other.nanos_since_epoch_;
}

private:
int64_t nanos_since_epoch_;
};
Expand Down
2 changes: 1 addition & 1 deletion api/include/opentelemetry/plugin/tracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class Span final : public trace::Span

void UpdateName(nostd::string_view name) noexcept override { span_->UpdateName(name); }

void End(const trace::EndSpanOptions &options = {}) noexcept override { span_->End(); }
void End(const trace::EndSpanOptions &options = {}) noexcept override { span_->End(options); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we have different structures for StartSpanOptions and EndSpanOptions?

Would it make sense to have one common options class, such as SpanOptions ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intent behind this design is to make it very clear to the API user which option can be set at which time (at start and end).

This also reflects the OpenTracing API, which provides FinishSpanOptions and StartSpanOptions.


bool IsRecording() const noexcept override { return span_->IsRecording(); }

Expand Down
2 changes: 1 addition & 1 deletion api/include/opentelemetry/trace/noop.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class NoopSpan final : public Span

void UpdateName(nostd::string_view /*name*/) noexcept override {}

void End(const EndSpanOptions &options = {}) noexcept override {}
void End(const EndSpanOptions & /*options*/) noexcept override {}

bool IsRecording() const noexcept override { return false; }

Expand Down
15 changes: 12 additions & 3 deletions api/include/opentelemetry/trace/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,13 @@ struct StartSpanOptions
// Attributes
SpanKind kind = SpanKind::kInternal;
};

/**
* StartEndOptions provides options to set properties of a Span when it is
* ended.
*/
struct EndSpanOptions
{
// Optionally sets the end time of a Span.
core::SteadyTimestamp end_steady_time;
};

Expand Down Expand Up @@ -135,8 +139,13 @@ class Span
// during creation.
virtual void UpdateName(nostd::string_view name) noexcept = 0;

// Mark the end of the Span. Only the timing of the first End call for a given Span will
// be recorded, and implementations are free to ignore all further calls.
/**
* Mark the end of the Span.
* Only the timing of the first End call for a given Span will be recorded,
* and implementations are free to ignore all further calls.
* @param options can be used to manually define span properties like the end
* timestamp
*/
virtual void End(const EndSpanOptions &options = {}) noexcept = 0;

// TODO
Expand Down
26 changes: 26 additions & 0 deletions api/test/core/timestamp_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ TEST(SystemTimestampTest, Construction)
t2.time_since_epoch());
}

TEST(SystemTimestampTest, Comparison)
{
SystemTimestamp t1;
SystemTimestamp t2;
SystemTimestamp t3{std::chrono::nanoseconds{2}};

EXPECT_EQ(t1, t1);
EXPECT_EQ(t1, t2);
EXPECT_EQ(t2, t1);
EXPECT_NE(t1, t3);
EXPECT_NE(t3, t1);
}

TEST(SteadyTimestampTest, Construction)
{
auto now_steady = std::chrono::steady_clock::now();
Expand All @@ -36,3 +49,16 @@ TEST(SteadyTimestampTest, Construction)
EXPECT_EQ(std::chrono::duration_cast<std::chrono::nanoseconds>(now_steady.time_since_epoch()),
t2.time_since_epoch());
}

TEST(SteadyTimestampTest, Comparison)
{
SteadyTimestamp t1;
SteadyTimestamp t2;
SteadyTimestamp t3{std::chrono::nanoseconds{2}};

EXPECT_EQ(t1, t1);
EXPECT_EQ(t1, t2);
EXPECT_EQ(t2, t1);
EXPECT_NE(t1, t3);
EXPECT_NE(t3, t1);
}
4 changes: 2 additions & 2 deletions examples/plugin/plugin/tracer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class Span final : public trace::Span
public:
Span(std::shared_ptr<Tracer> &&tracer,
nostd::string_view name,
const trace::StartSpanOptions &options) noexcept
const trace::StartSpanOptions & /*options*/) noexcept
: tracer_{std::move(tracer)}, name_{name}
{
std::cout << "StartSpan: " << name << "\n";
Expand All @@ -38,7 +38,7 @@ class Span final : public trace::Span

void UpdateName(nostd::string_view /*name*/) noexcept override {}

void End(const trace::EndSpanOptions &options) noexcept override {}
void End(const trace::EndSpanOptions & /*options*/) noexcept override {}

bool IsRecording() const noexcept override { return true; }

Expand Down
15 changes: 8 additions & 7 deletions sdk/src/trace/span.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ using opentelemetry::core::SystemTimestamp;

namespace
{
SystemTimestamp NowOrGiven(const SystemTimestamp &system)
SystemTimestamp NowOr(const SystemTimestamp &system)
{
if (system == SystemTimestamp())
{
Expand All @@ -25,7 +25,7 @@ SystemTimestamp NowOrGiven(const SystemTimestamp &system)
}
}

SteadyTimestamp NowOrGiven(const SteadyTimestamp &steady)
SteadyTimestamp NowOr(const SteadyTimestamp &steady)
{
if (steady == SteadyTimestamp())
{
Expand Down Expand Up @@ -55,8 +55,8 @@ Span::Span(std::shared_ptr<Tracer> &&tracer,
processor_->OnStart(*recordable_);
recordable_->SetName(name);

recordable_->SetStartTime(NowOrGiven(options.start_system_time));
start_steady_time = NowOrGiven(options.start_steady_time);
recordable_->SetStartTime(NowOr(options.start_system_time));
start_steady_time = NowOr(options.start_steady_time);
}

Span::~Span()
Expand Down Expand Up @@ -112,9 +112,10 @@ void Span::End(const trace_api::EndSpanOptions &options) noexcept
return;
}

recordable_->SetDuration(
std::chrono::steady_clock::time_point(NowOrGiven(options.end_steady_time)) -
std::chrono::steady_clock::time_point(start_steady_time));
auto end_steady_time = NowOr(options.end_steady_time);
recordable_->SetDuration(std::chrono::steady_clock::time_point(end_steady_time) -
std::chrono::steady_clock::time_point(start_steady_time));

processor_->OnEnd(std::move(recordable_));
recordable_.reset();
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/test/trace/tracer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ TEST(Tracer, StartSpan)
ASSERT_LT(std::chrono::nanoseconds(0), span_data->GetDuration());
}

TEST(Tracer, StartSpanWithTime)
TEST(Tracer, StartSpanWithOptions)
{
std::shared_ptr<std::vector<std::unique_ptr<SpanData>>> spans_received(
new std::vector<std::unique_ptr<SpanData>>);
Expand Down