Skip to content

Commit

Permalink
[PROTO] Upgrade to opentelemetry-proto v1.1.0 (#2488)
Browse files Browse the repository at this point in the history
  • Loading branch information
marcalff authored Jan 16, 2024
1 parent c4f39f2 commit 423ecac
Show file tree
Hide file tree
Showing 19 changed files with 111 additions and 12 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ Increment the:
[#2450](https://github.com/open-telemetry/opentelemetry-cpp/pull/2450)
* [EXPORTER] Set `is_monotonic` flag for Observable Counters
[#2478](https://github.com/open-telemetry/opentelemetry-cpp/pull/2478)
* [PROTO] Upgrade to opentelemetry-proto v1.1.0
[#2488](https://github.com/open-telemetry/opentelemetry-cpp/pull/2488)

Important changes:

Expand Down
15 changes: 15 additions & 0 deletions api/include/opentelemetry/trace/trace_flags.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,28 @@ class TraceFlags final
{
public:
static constexpr uint8_t kIsSampled = 1;
static constexpr uint8_t kIsRandom = 2;

/**
* Valid flags in W3C Trace Context version 1.
* See https://www.w3.org/TR/trace-context-1/#trace-flags
*/
static constexpr uint8_t kAllW3CTraceContext1Flags = kIsSampled;

/**
* Valid flags in W3C Trace Context version 2.
* See https://www.w3.org/TR/trace-context-1/#trace-flags
*/
static constexpr uint8_t kAllW3CTraceContext2Flags = kIsSampled | kIsRandom;

TraceFlags() noexcept : rep_{0} {}

explicit TraceFlags(uint8_t flags) noexcept : rep_(flags) {}

bool IsSampled() const noexcept { return rep_ & kIsSampled; }

bool IsRandom() const noexcept { return rep_ & kIsRandom; }

// Populates the buffer with the lowercase base16 representation of the flags.
void ToLowerBase16(nostd::span<char, 2> buffer) const noexcept
{
Expand Down
6 changes: 3 additions & 3 deletions bazel/repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ def opentelemetry_cpp_deps():
http_archive,
name = "com_github_opentelemetry_proto",
build_file = "@io_opentelemetry_cpp//bazel:opentelemetry_proto.BUILD",
sha256 = "a13a1a7b76a1f22a0ca2e6c293e176ffef031413ab8ba653a82a1dbc286a3a33",
strip_prefix = "opentelemetry-proto-1.0.0",
sha256 = "df491a05f3fcbf86cc5ba5c9de81f6a624d74d4773d7009d573e37d6e2b6af64",
strip_prefix = "opentelemetry-proto-1.1.0",
urls = [
"https://github.com/open-telemetry/opentelemetry-proto/archive/v1.0.0.tar.gz",
"https://github.com/open-telemetry/opentelemetry-proto/archive/v1.1.0.tar.gz",
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ namespace trace

class ETWRandomIdGenerator : public IdGenerator
{
public:
ETWRandomIdGenerator() : IdGenerator(false) {}

opentelemetry::trace::SpanId GenerateSpanId() noexcept override
{
Expand Down
3 changes: 3 additions & 0 deletions exporters/etw/test/etw_tracer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ std::string getTemporaryValue()
*/
class MockIdGenerator : public sdk::trace::IdGenerator
{
public:
MockIdGenerator() : sdk::trace::IdGenerator(false) {}

opentelemetry::trace::SpanId GenerateSpanId() noexcept override
{
return opentelemetry::trace::SpanId(buf_span);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ class OtlpRecordable final : public opentelemetry::sdk::trace::Recordable

void SetName(nostd::string_view name) noexcept override;

void SetTraceFlags(opentelemetry::trace::TraceFlags flags) noexcept override;

void SetSpanKind(opentelemetry::trace::SpanKind span_kind) noexcept override;

void SetResource(const opentelemetry::sdk::resource::Resource &resource) noexcept override;
Expand Down
7 changes: 7 additions & 0 deletions exporters/otlp/src/otlp_recordable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,13 @@ void OtlpRecordable::SetName(nostd::string_view name) noexcept
span_.set_name(name.data(), name.size());
}

void OtlpRecordable::SetTraceFlags(opentelemetry::trace::TraceFlags flags) noexcept
{
uint32_t all_flags = flags.flags() & opentelemetry::proto::trace::v1::SPAN_FLAGS_TRACE_FLAGS_MASK;

span_.set_flags(all_flags);
}

void OtlpRecordable::SetSpanKind(trace::SpanKind span_kind) noexcept
{
proto::trace::v1::Span_SpanKind proto_span_kind =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class Recordable final : public sdk::trace::Recordable

void SetName(nostd::string_view name) noexcept override;

void SetTraceFlags(opentelemetry::trace::TraceFlags flags) noexcept override;

void SetStartTime(opentelemetry::common::SystemTimestamp start_time) noexcept override;

void SetSpanKind(opentelemetry::trace::SpanKind span_kind) noexcept override;
Expand Down
6 changes: 6 additions & 0 deletions exporters/zipkin/src/recordable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,12 @@ void Recordable::SetName(nostd::string_view name) noexcept
span_["name"] = name.data();
}

void Recordable::SetTraceFlags(opentelemetry::trace::TraceFlags /* flags */) noexcept
{
// TODO: Currently not supported by specs:
// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/zipkin.md
}

void Recordable::SetResource(const sdk::resource::Resource &resource) noexcept
{
// only service.name attribute is supported by specs as of now.
Expand Down
11 changes: 10 additions & 1 deletion sdk/include/opentelemetry/sdk/trace/id_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,24 @@ namespace trace
/** IdGenerator provides an interface for generating Trace Id and Span Id */
class IdGenerator
{

public:
IdGenerator(bool is_random) : is_random_(is_random) {}

virtual ~IdGenerator() = default;

/** Returns a SpanId represented by opaque 128-bit trace identifier */
virtual opentelemetry::trace::SpanId GenerateSpanId() noexcept = 0;

/** Returns a TraceId represented by opaque 64-bit trace identifier */
virtual opentelemetry::trace::TraceId GenerateTraceId() noexcept = 0;

bool IsRandom() const { return is_random_; }

private:
/** True if GenerateTraceId() is random,
* per https://www.w3.org/TR/trace-context-2/#random-trace-id-flag
*/
bool is_random_;
};
} // namespace trace

Expand Down
8 changes: 8 additions & 0 deletions sdk/include/opentelemetry/sdk/trace/multi_recordable.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,14 @@ class MultiRecordable : public Recordable
}
}

void SetTraceFlags(opentelemetry::trace::TraceFlags flags) noexcept override
{
for (auto &recordable : recordables_)
{
recordable.second->SetTraceFlags(flags);
}
}

void SetSpanKind(opentelemetry::trace::SpanKind span_kind) noexcept override
{
for (auto &recordable : recordables_)
Expand Down
2 changes: 2 additions & 0 deletions sdk/include/opentelemetry/sdk/trace/random_id_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ namespace trace
class RandomIdGenerator : public IdGenerator
{
public:
RandomIdGenerator() : IdGenerator(true) {}

opentelemetry::trace::SpanId GenerateSpanId() noexcept override;

opentelemetry::trace::TraceId GenerateTraceId() noexcept override;
Expand Down
6 changes: 6 additions & 0 deletions sdk/include/opentelemetry/sdk/trace/recordable.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ class Recordable
*/
virtual void SetName(nostd::string_view name) noexcept = 0;

/**
* Set the trace flags of the span.
* @param flags the flags to set
*/
virtual void SetTraceFlags(opentelemetry::trace::TraceFlags flags) noexcept = 0;

This comment has been minimized.

Copy link
@dbolduc

dbolduc Feb 19, 2024

Contributor

This was a breaking change. Why not provide a no-op default implementation?

  virtual void SetTraceFlags(opentelemetry::trace::TraceFlags flags) noexcept {};

/**
* Set the spankind of the span.
* @param span_kind the spankind to set
Expand Down
9 changes: 9 additions & 0 deletions sdk/include/opentelemetry/sdk/trace/span_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ class SpanData final : public Recordable
*/
opentelemetry::nostd::string_view GetName() const noexcept { return name_; }

/**
* Get the trace flags for this span
* @return the trace flags for this span
*/
opentelemetry::trace::TraceFlags GetFlags() const noexcept { return flags_; }

/**
* Get the kind of this span
* @return the kind of this span
Expand Down Expand Up @@ -273,6 +279,8 @@ class SpanData final : public Recordable
name_ = std::string(name.data(), name.length());
}

void SetTraceFlags(opentelemetry::trace::TraceFlags flags) noexcept override { flags_ = flags; }

void SetSpanKind(opentelemetry::trace::SpanKind span_kind) noexcept override
{
span_kind_ = span_kind;
Expand Down Expand Up @@ -306,6 +314,7 @@ class SpanData final : public Recordable
opentelemetry::sdk::common::AttributeMap attribute_map_;
std::vector<SpanDataEvent> events_;
std::vector<SpanDataLink> links_;
opentelemetry::trace::TraceFlags flags_;
opentelemetry::trace::SpanKind span_kind_{opentelemetry::trace::SpanKind::kInternal};
const opentelemetry::sdk::resource::Resource *resource_;
const InstrumentationScope *instrumentation_scope_;
Expand Down
2 changes: 2 additions & 0 deletions sdk/src/trace/span.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ Span::Span(std::shared_ptr<Tracer> &&tracer,
? parent_span_context.span_id()
: opentelemetry::trace::SpanId());

recordable_->SetTraceFlags(span_context_->trace_flags());

attributes.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept {
recordable_->SetAttribute(key, value);
return true;
Expand Down
33 changes: 27 additions & 6 deletions sdk/src/trace/tracer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,26 +52,47 @@ nostd::shared_ptr<opentelemetry::trace::Span> Tracer::StartSpan(
}
}

IdGenerator &generator = GetIdGenerator();
opentelemetry::trace::TraceId trace_id;
opentelemetry::trace::SpanId span_id = GetIdGenerator().GenerateSpanId();
opentelemetry::trace::SpanId span_id = generator.GenerateSpanId();
bool is_parent_span_valid = false;
uint8_t flags = 0;

if (parent_context.IsValid())
{
trace_id = parent_context.trace_id();
flags = parent_context.trace_flags().flags();
is_parent_span_valid = true;
}
else
{
trace_id = GetIdGenerator().GenerateTraceId();
trace_id = generator.GenerateTraceId();
if (generator.IsRandom())
{
flags = opentelemetry::trace::TraceFlags::kIsRandom;
}
}

auto sampling_result = context_->GetSampler().ShouldSample(parent_context, trace_id, name,
options.kind, attributes, links);
auto trace_flags =
sampling_result.IsSampled()
? opentelemetry::trace::TraceFlags{opentelemetry::trace::TraceFlags::kIsSampled}
: opentelemetry::trace::TraceFlags{};
if (sampling_result.IsSampled())
{
flags |= opentelemetry::trace::TraceFlags::kIsSampled;
}

#if 1
/* https://github.com/open-telemetry/opentelemetry-specification as of v1.29.0 */
/* Support W3C Trace Context version 1. */
flags &= opentelemetry::trace::TraceFlags::kAllW3CTraceContext1Flags;
#endif

#if 0
/* Waiting for https://github.com/open-telemetry/opentelemetry-specification/issues/3411 */
/* Support W3C Trace Context version 2. */
flags &= opentelemetry::trace::TraceFlags::kAllW3CTraceContext2Flags;
#endif

opentelemetry::trace::TraceFlags trace_flags(flags);

auto span_context =
std::unique_ptr<opentelemetry::trace::SpanContext>(new opentelemetry::trace::SpanContext(
Expand Down
3 changes: 3 additions & 0 deletions sdk/test/trace/tracer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class MockSampler final : public Sampler
*/
class MockIdGenerator : public IdGenerator
{
public:
MockIdGenerator() : IdGenerator(false) {}

opentelemetry::trace::SpanId GenerateSpanId() noexcept override
{
return opentelemetry::trace::SpanId(buf_span);
Expand Down
2 changes: 1 addition & 1 deletion third_party_release
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ benchmark=v1.7.1
googletest=1.13.0
ms-gsl=v3.1.0-67-g6f45293
nlohmann-json=v3.11.2
opentelemetry-proto=v1.0.0
opentelemetry-proto=v1.1.0
opentracing-cpp=v1.6.0
prometheus-cpp=v1.1.0
vcpkg=2022.08.15

2 comments on commit 423ecac

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 423ecac Previous: c4f39f2 Ratio
BM_BaselineBuffer/1 8754103.183746338 ns/iter 568410.8734130859 ns/iter 15.40
BM_BaselineBuffer/2 9320435.523986816 ns/iter 2852612.4954223633 ns/iter 3.27
BM_BaselineBuffer/4 14513101.577758789 ns/iter 2351486.6317290603 ns/iter 6.17
BM_LockFreeBuffer/2 7199919.2237854 ns/iter 1024306.7741394043 ns/iter 7.03
BM_LockFreeBuffer/4 10236163.139343262 ns/iter 1083777.2313286276 ns/iter 9.44

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp api Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 423ecac Previous: c4f39f2 Ratio
BM_SpinLockThrashing/1/process_time/real_time 3.518400192260742 ms/iter 0.08849987334028307 ms/iter 39.76
BM_NaiveSpinLockThrashing/1/process_time/real_time 0.20251154899597168 ms/iter 0.07304619232894695 ms/iter 2.77
BM_NaiveSpinLockThrashing/2/process_time/real_time 1.0912227630615234 ms/iter 0.1429436012841646 ms/iter 7.63

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.