From d19462ed2cddbaf23bc04648d2ac9c271b52c57e Mon Sep 17 00:00:00 2001 From: zirain Date: Mon, 27 May 2024 10:32:27 +0800 Subject: [PATCH] rename getTraceIdAsHex to getTraceId (#34314) * rename getTraceId Signed-off-by: zirain * implement getTraceId Signed-off-by: zirain * fix test Signed-off-by: zirain * fix test Signed-off-by: zirain --------- Signed-off-by: zirain --- envoy/tracing/trace_driver.h | 6 +++--- source/common/formatter/http_specific_formatter.cc | 4 ++-- source/common/tracing/null_span_impl.h | 2 +- .../access_loggers/open_telemetry/access_log_impl.cc | 2 +- .../extensions/tracers/common/ot/opentracing_driver_impl.h | 4 +++- source/extensions/tracers/datadog/span.cc | 2 +- source/extensions/tracers/datadog/span.h | 2 +- .../tracers/opencensus/opencensus_tracer_impl.cc | 4 ++-- source/extensions/tracers/opentelemetry/tracer.cc | 7 +++---- source/extensions/tracers/opentelemetry/tracer.h | 2 +- source/extensions/tracers/skywalking/tracer.h | 2 +- source/extensions/tracers/xray/tracer.h | 2 +- source/extensions/tracers/zipkin/zipkin_tracer_impl.h | 2 +- test/common/formatter/substitution_formatter_test.cc | 3 +-- test/common/tracing/tracer_impl_test.cc | 2 +- .../access_loggers/open_telemetry/access_log_impl_test.cc | 4 ++-- .../extensions/filters/http/ext_proc/tracer_test_filter.cc | 2 +- .../tracers/common/ot/opentracing_driver_impl_test.cc | 2 +- test/extensions/tracers/datadog/span_test.cc | 6 +++--- test/extensions/tracers/opencensus/tracer_test.cc | 4 ++-- .../opentelemetry/opentelemetry_tracer_impl_test.cc | 2 +- test/extensions/tracers/skywalking/tracer_test.cc | 3 +-- test/extensions/tracers/xray/tracer_test.cc | 4 ++-- test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc | 4 ++-- test/mocks/tracing/mocks.h | 2 +- 25 files changed, 39 insertions(+), 40 deletions(-) diff --git a/envoy/tracing/trace_driver.h b/envoy/tracing/trace_driver.h index d57f0a61a630..60e25ec84a68 100644 --- a/envoy/tracing/trace_driver.h +++ b/envoy/tracing/trace_driver.h @@ -15,7 +15,7 @@ class Span; using SpanPtr = std::unique_ptr; /** - * The upstream sevice type. + * The upstream service type. */ enum class ServiceType { // Service type is unknown. @@ -133,9 +133,9 @@ class Span { * Retrieve the trace ID associated with this span. * The trace id may be generated for this span, propagated by parent spans, or * not created yet. - * @return trace ID as a hex string + * @return trace ID */ - virtual std::string getTraceIdAsHex() const PURE; + virtual std::string getTraceId() const PURE; }; /** diff --git a/source/common/formatter/http_specific_formatter.cc b/source/common/formatter/http_specific_formatter.cc index 7623c4b95ac9..7f6376224b79 100644 --- a/source/common/formatter/http_specific_formatter.cc +++ b/source/common/formatter/http_specific_formatter.cc @@ -199,7 +199,7 @@ HeadersByteSizeFormatter::formatValueWithContext(const HttpFormatterContext& con ProtobufWkt::Value TraceIDFormatter::formatValueWithContext(const HttpFormatterContext& context, const StreamInfo::StreamInfo&) const { - auto trace_id = context.activeSpan().getTraceIdAsHex(); + auto trace_id = context.activeSpan().getTraceId(); if (trace_id.empty()) { return SubstitutionFormatUtils::unspecifiedValue(); } @@ -209,7 +209,7 @@ ProtobufWkt::Value TraceIDFormatter::formatValueWithContext(const HttpFormatterC absl::optional TraceIDFormatter::formatWithContext(const HttpFormatterContext& context, const StreamInfo::StreamInfo&) const { - auto trace_id = context.activeSpan().getTraceIdAsHex(); + auto trace_id = context.activeSpan().getTraceId(); if (trace_id.empty()) { return absl::nullopt; } diff --git a/source/common/tracing/null_span_impl.h b/source/common/tracing/null_span_impl.h index 109fce23d961..d573318e5751 100644 --- a/source/common/tracing/null_span_impl.h +++ b/source/common/tracing/null_span_impl.h @@ -25,7 +25,7 @@ class NullSpan : public Span { void injectContext(Tracing::TraceContext&, const UpstreamContext&) override {} void setBaggage(absl::string_view, absl::string_view) override {} std::string getBaggage(absl::string_view) override { return EMPTY_STRING; } - std::string getTraceIdAsHex() const override { return EMPTY_STRING; } + std::string getTraceId() const override { return EMPTY_STRING; } SpanPtr spawnChild(const Config&, const std::string&, SystemTime) override { return SpanPtr{new NullSpan()}; } diff --git a/source/extensions/access_loggers/open_telemetry/access_log_impl.cc b/source/extensions/access_loggers/open_telemetry/access_log_impl.cc index 484bfd73a8b8..e3866dfc98e7 100644 --- a/source/extensions/access_loggers/open_telemetry/access_log_impl.cc +++ b/source/extensions/access_loggers/open_telemetry/access_log_impl.cc @@ -95,7 +95,7 @@ void AccessLog::emitLog(const Formatter::HttpFormatterContext& log_context, // OpenTelemetry trace id is a [16]byte array, backend(e.g. OTel-collector) will reject the // request if the length is not 16. Some trace provider(e.g. zipkin) may return it as a 64-bit hex // string. In this case, we need to convert it to a 128-bit hex string, padding left with zeros. - std::string trace_id_hex = log_context.activeSpan().getTraceIdAsHex(); + std::string trace_id_hex = log_context.activeSpan().getTraceId(); if (trace_id_hex.size() == 32) { *log_entry.mutable_trace_id() = absl::HexStringToBytes(trace_id_hex); } else if (trace_id_hex.size() == 16) { diff --git a/source/extensions/tracers/common/ot/opentracing_driver_impl.h b/source/extensions/tracers/common/ot/opentracing_driver_impl.h index 5ad651d43bf9..5fd46bfe96a4 100644 --- a/source/extensions/tracers/common/ot/opentracing_driver_impl.h +++ b/source/extensions/tracers/common/ot/opentracing_driver_impl.h @@ -46,7 +46,9 @@ class OpenTracingSpan : public Tracing::Span, Logger::Loggable span_; diff --git a/source/extensions/tracers/opencensus/opencensus_tracer_impl.cc b/source/extensions/tracers/opencensus/opencensus_tracer_impl.cc index 19c870c38581..026f9e70f4cf 100644 --- a/source/extensions/tracers/opencensus/opencensus_tracer_impl.cc +++ b/source/extensions/tracers/opencensus/opencensus_tracer_impl.cc @@ -80,7 +80,7 @@ class Span : public Tracing::Span { void setBaggage(absl::string_view, absl::string_view) override{}; std::string getBaggage(absl::string_view) override { return EMPTY_STRING; }; - std::string getTraceIdAsHex() const override; + std::string getTraceId() const override; private: ::opencensus::trace::Span span_; @@ -236,7 +236,7 @@ void Span::injectContext(Tracing::TraceContext& trace_context, const Tracing::Up } } -std::string Span::getTraceIdAsHex() const { +std::string Span::getTraceId() const { const auto& ctx = span_.context(); return ctx.trace_id().ToHex(); } diff --git a/source/extensions/tracers/opentelemetry/tracer.cc b/source/extensions/tracers/opentelemetry/tracer.cc index aa9307578ce3..5c755bf873f9 100644 --- a/source/extensions/tracers/opentelemetry/tracer.cc +++ b/source/extensions/tracers/opentelemetry/tracer.cc @@ -39,9 +39,8 @@ void callSampler(SamplerSharedPtr sampler, const absl::optional spa if (!sampler) { return; } - const auto sampling_result = - sampler->shouldSample(span_context, new_span.getTraceIdAsHex(), operation_name, - new_span.spankind(), trace_context, {}); + const auto sampling_result = sampler->shouldSample( + span_context, new_span.getTraceId(), operation_name, new_span.spankind(), trace_context, {}); new_span.setSampled(sampling_result.isSampled()); if (sampling_result.attributes) { @@ -70,7 +69,7 @@ Span::Span(const std::string& name, SystemTime start_time, Envoy::TimeSource& ti Tracing::SpanPtr Span::spawnChild(const Tracing::Config&, const std::string& name, SystemTime start_time) { // Build span_context from the current span, then generate the child span from that context. - SpanContext span_context(kDefaultVersion, getTraceIdAsHex(), spanId(), sampled(), tracestate()); + SpanContext span_context(kDefaultVersion, getTraceId(), spanId(), sampled(), tracestate()); return parent_tracer_.startSpan(name, start_time, span_context, {}, ::opentelemetry::proto::trace::v1::Span::SPAN_KIND_CLIENT); } diff --git a/source/extensions/tracers/opentelemetry/tracer.h b/source/extensions/tracers/opentelemetry/tracer.h index 2c9bb216b411..057814365b9a 100644 --- a/source/extensions/tracers/opentelemetry/tracer.h +++ b/source/extensions/tracers/opentelemetry/tracer.h @@ -117,7 +117,7 @@ class Span : Logger::Loggable, public Tracing::Span { span_.set_trace_id(absl::HexStringToBytes(trace_id_hex)); } - std::string getTraceIdAsHex() const override { return absl::BytesToHexString(span_.trace_id()); }; + std::string getTraceId() const override { return absl::BytesToHexString(span_.trace_id()); }; OTelSpanKind spankind() const { return span_.kind(); } diff --git a/source/extensions/tracers/skywalking/tracer.h b/source/extensions/tracers/skywalking/tracer.h index 0df23cf899b0..d5bb1f65f329 100644 --- a/source/extensions/tracers/skywalking/tracer.h +++ b/source/extensions/tracers/skywalking/tracer.h @@ -88,7 +88,7 @@ class Span : public Tracing::Span { void setSampled(bool do_sample) override; std::string getBaggage(absl::string_view) override { return EMPTY_STRING; } void setBaggage(absl::string_view, absl::string_view) override {} - std::string getTraceIdAsHex() const override { return EMPTY_STRING; } + std::string getTraceId() const override { return tracing_context_->traceId(); } const TracingContextPtr tracingContext() { return tracing_context_; } const TracingSpanPtr spanEntity() { return span_entity_; } diff --git a/source/extensions/tracers/xray/tracer.h b/source/extensions/tracers/xray/tracer.h index 4ed4a7533638..bf9d696c0836 100644 --- a/source/extensions/tracers/xray/tracer.h +++ b/source/extensions/tracers/xray/tracer.h @@ -231,7 +231,7 @@ class Span : public Tracing::Span, Logger::Loggable { std::string getBaggage(absl::string_view) override { return EMPTY_STRING; } // TODO: This method is unimplemented for X-Ray. - std::string getTraceIdAsHex() const override { return EMPTY_STRING; }; + std::string getTraceId() const override { return trace_id_; }; /** * Creates a child span. diff --git a/source/extensions/tracers/zipkin/zipkin_tracer_impl.h b/source/extensions/tracers/zipkin/zipkin_tracer_impl.h index 9ae05d474f78..cfa3c95a4fc3 100644 --- a/source/extensions/tracers/zipkin/zipkin_tracer_impl.h +++ b/source/extensions/tracers/zipkin/zipkin_tracer_impl.h @@ -83,7 +83,7 @@ class ZipkinSpan : public Tracing::Span { void setBaggage(absl::string_view, absl::string_view) override; std::string getBaggage(absl::string_view) override; - std::string getTraceIdAsHex() const override { return span_.traceIdAsHexString(); }; + std::string getTraceId() const override { return span_.traceIdAsHexString(); }; /** * @return a reference to the Zipkin::Span object. diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index ef0b1704d9b4..36442dc946be 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -2153,8 +2153,7 @@ TEST(SubstitutionFormatterTest, TraceIDFormatter) { std::string body; Tracing::MockSpan active_span; - EXPECT_CALL(active_span, getTraceIdAsHex()) - .WillRepeatedly(Return("ae0046f9075194306d7de2931bd38ce3")); + EXPECT_CALL(active_span, getTraceId()).WillRepeatedly(Return("ae0046f9075194306d7de2931bd38ce3")); { HttpFormatterContext formatter_context(&request_header, &response_header, &response_trailer, diff --git a/test/common/tracing/tracer_impl_test.cc b/test/common/tracing/tracer_impl_test.cc index 8a89138ad5de..851795817631 100644 --- a/test/common/tracing/tracer_impl_test.cc +++ b/test/common/tracing/tracer_impl_test.cc @@ -264,7 +264,7 @@ TEST(NullTracerTest, BasicFunctionality) { span_ptr->setTag("foo", "bar"); span_ptr->setBaggage("key", "value"); ASSERT_EQ("", span_ptr->getBaggage("baggage_key")); - ASSERT_EQ(span_ptr->getTraceIdAsHex(), ""); + ASSERT_EQ(span_ptr->getTraceId(), ""); span_ptr->injectContext(trace_context, upstream_context); span_ptr->log(SystemTime(), "fake_event"); diff --git a/test/extensions/access_loggers/open_telemetry/access_log_impl_test.cc b/test/extensions/access_loggers/open_telemetry/access_log_impl_test.cc index 0a163b933c94..b16d47d135c2 100644 --- a/test/extensions/access_loggers/open_telemetry/access_log_impl_test.cc +++ b/test/extensions/access_loggers/open_telemetry/access_log_impl_test.cc @@ -175,7 +175,7 @@ TEST_F(AccessLogTest, TraceId) { NiceMock active_span; - EXPECT_CALL(active_span, getTraceIdAsHex()).WillOnce(Return("404142434445464748494a4b4c4d4e4f")); + EXPECT_CALL(active_span, getTraceId()).WillOnce(Return("404142434445464748494a4b4c4d4e4f")); expectLog(R"EOF( trace_id: "QEFCQ0RFRkdISUpLTE1OTw==" time_unix_nano: 3600000000000 @@ -195,7 +195,7 @@ TEST_F(AccessLogTest, ZipkinTraceId) { NiceMock active_span; - EXPECT_CALL(active_span, getTraceIdAsHex()).WillOnce(Return("0ccce09bf12e94df")); + EXPECT_CALL(active_span, getTraceId()).WillOnce(Return("0ccce09bf12e94df")); expectLog(R"EOF( trace_id: "AAAAAAAAAAAMzOCb8S6U3w==" time_unix_nano: 3600000000000 diff --git a/test/extensions/filters/http/ext_proc/tracer_test_filter.cc b/test/extensions/filters/http/ext_proc/tracer_test_filter.cc index ec59c159c555..d58c97372160 100644 --- a/test/extensions/filters/http/ext_proc/tracer_test_filter.cc +++ b/test/extensions/filters/http/ext_proc/tracer_test_filter.cc @@ -69,7 +69,7 @@ class Span : public Tracing::Span { /* not implemented */ return EMPTY_STRING; }; - std::string getTraceIdAsHex() const { + std::string getTraceId() const { /* not implemented */ return EMPTY_STRING; }; diff --git a/test/extensions/tracers/common/ot/opentracing_driver_impl_test.cc b/test/extensions/tracers/common/ot/opentracing_driver_impl_test.cc index 8f88b9b1a632..99c2ef75712e 100644 --- a/test/extensions/tracers/common/ot/opentracing_driver_impl_test.cc +++ b/test/extensions/tracers/common/ot/opentracing_driver_impl_test.cc @@ -352,7 +352,7 @@ TEST_F(OpenTracingDriverTest, GetTraceId) { first_span->finishSpan(); // This method is unimplemented and a noop. - ASSERT_EQ(first_span->getTraceIdAsHex(), ""); + ASSERT_EQ(first_span->getTraceId(), ""); } TEST_F(OpenTracingDriverTest, ExtractUsingForeach) { diff --git a/test/extensions/tracers/datadog/span_test.cc b/test/extensions/tracers/datadog/span_test.cc index f4294ab120ed..2dccafbea424 100644 --- a/test/extensions/tracers/datadog/span_test.cc +++ b/test/extensions/tracers/datadog/span_test.cc @@ -386,9 +386,9 @@ TEST_F(DatadogTracerSpanTest, Baggage) { EXPECT_EQ("", span.getBaggage("foo")); } -TEST_F(DatadogTracerSpanTest, GetTraceIdAsHex) { +TEST_F(DatadogTracerSpanTest, GetTraceId) { Span span{std::move(span_)}; - EXPECT_EQ("cafebabe", span.getTraceIdAsHex()); + EXPECT_EQ("cafebabe", span.getTraceId()); } TEST_F(DatadogTracerSpanTest, NoOpMode) { @@ -429,7 +429,7 @@ TEST_F(DatadogTracerSpanTest, NoOpMode) { span.setSampled(false); EXPECT_EQ("", span.getBaggage("foo")); span.setBaggage("foo", "bar"); - EXPECT_EQ("", span.getTraceIdAsHex()); + EXPECT_EQ("", span.getTraceId()); } } // namespace diff --git a/test/extensions/tracers/opencensus/tracer_test.cc b/test/extensions/tracers/opencensus/tracer_test.cc index 0811f097e035..65a1632596f6 100644 --- a/test/extensions/tracers/opencensus/tracer_test.cc +++ b/test/extensions/tracers/opencensus/tracer_test.cc @@ -129,7 +129,7 @@ TEST(OpenCensusTracerTest, Span) { ASSERT_EQ("", span->getBaggage("baggage_key")); // Trace id is automatically created when no parent context exists. - ASSERT_NE(span->getTraceIdAsHex(), ""); + ASSERT_NE(span->getTraceId(), ""); } // Retrieve SpanData from the OpenCensus trace exporter. @@ -221,7 +221,7 @@ void testIncomingHeaders( // Check contents via public API. // Trace id is set via context propagation headers. - EXPECT_EQ(span->getTraceIdAsHex(), "404142434445464748494a4b4c4d4e4f"); + EXPECT_EQ(span->getTraceId(), "404142434445464748494a4b4c4d4e4f"); } // Retrieve SpanData from the OpenCensus trace exporter. diff --git a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc index cb0227604366..1be53b67e720 100644 --- a/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc +++ b/test/extensions/tracers/opentelemetry/opentelemetry_tracer_impl_test.cc @@ -175,7 +175,7 @@ TEST_F(OpenTelemetryDriverTest, ParseSpanContextFromHeadersTest) { Tracing::SpanPtr span = driver_->startSpan(mock_tracing_config_, request_headers, stream_info_, operation_name_, {Tracing::Reason::Sampling, true}); - EXPECT_EQ(span->getTraceIdAsHex(), trace_id_hex); + EXPECT_EQ(span->getTraceId(), trace_id_hex); // Remove headers, then inject context into header from the span. request_headers.remove(OpenTelemetryConstants::get().TRACE_PARENT.key()); diff --git a/test/extensions/tracers/skywalking/tracer_test.cc b/test/extensions/tracers/skywalking/tracer_test.cc index d0f22d1c3746..ecb4183355df 100644 --- a/test/extensions/tracers/skywalking/tracer_test.cc +++ b/test/extensions/tracers/skywalking/tracer_test.cc @@ -88,8 +88,7 @@ TEST_F(TracerTest, TracerTestCreateNewSpanWithNoPropagationHeaders) { EXPECT_EQ("", span->getBaggage("FakeStringAndNothingToDo")); span->setOperation("FakeStringAndNothingToDo"); span->setBaggage("FakeStringAndNothingToDo", "FakeStringAndNothingToDo"); - // This method is unimplemented and a noop. - ASSERT_EQ(span->getTraceIdAsHex(), ""); + ASSERT_EQ(span->getTraceId(), segment_context->traceId()); // Test whether the basic functions of Span are normal. EXPECT_FALSE(span->spanEntity()->skipAnalysis()); span->setSampled(false); diff --git a/test/extensions/tracers/xray/tracer_test.cc b/test/extensions/tracers/xray/tracer_test.cc index ffda75fe1301..12b7c50238f9 100644 --- a/test/extensions/tracers/xray/tracer_test.cc +++ b/test/extensions/tracers/xray/tracer_test.cc @@ -386,8 +386,8 @@ TEST_F(XRayTracerTest, GetTraceId) { auto span = tracer.createNonSampledSpan(absl::nullopt /*headers*/); span->finishSpan(); - // This method is unimplemented and a noop. - EXPECT_EQ(span->getTraceIdAsHex(), ""); + // Trace ID is always generated + EXPECT_NE(span->getTraceId(), ""); } TEST_F(XRayTracerTest, ChildSpanHasParentInfo) { diff --git a/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc b/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc index 0269ec966cd9..384d53bee290 100644 --- a/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc +++ b/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc @@ -725,7 +725,7 @@ TEST_F(ZipkinDriverTest, ZipkinSpanTest) { // ==== Tracing::SpanPtr span6 = driver_->startSpan(config_, request_headers_, stream_info_, operation_name_, {Tracing::Reason::Sampling, true}); - EXPECT_EQ(span6->getTraceIdAsHex(), "0000000000000000"); + EXPECT_EQ(span6->getTraceId(), "0000000000000000"); } TEST_F(ZipkinDriverTest, ZipkinSpanContextFromB3HeadersTest) { @@ -798,7 +798,7 @@ TEST_F(ZipkinDriverTest, ZipkinSpanContextFromB3Headers128TraceIdTest) { EXPECT_EQ(span_id, zipkin_span->span().idAsHexString()); EXPECT_EQ(parent_id, zipkin_span->span().parentIdAsHexString()); EXPECT_TRUE(zipkin_span->span().sampled()); - EXPECT_EQ(trace_id, zipkin_span->getTraceIdAsHex()); + EXPECT_EQ(trace_id, zipkin_span->getTraceId()); } TEST_F(ZipkinDriverTest, ZipkinSpanContextFromInvalidTraceIdB3HeadersTest) { diff --git a/test/mocks/tracing/mocks.h b/test/mocks/tracing/mocks.h index 90905dbf1045..063902b3aadf 100644 --- a/test/mocks/tracing/mocks.h +++ b/test/mocks/tracing/mocks.h @@ -43,7 +43,7 @@ class MockSpan : public Span { MOCK_METHOD(void, setSampled, (const bool sampled)); MOCK_METHOD(void, setBaggage, (absl::string_view key, absl::string_view value)); MOCK_METHOD(std::string, getBaggage, (absl::string_view key)); - MOCK_METHOD(std::string, getTraceIdAsHex, (), (const)); + MOCK_METHOD(std::string, getTraceId, (), (const)); SpanPtr spawnChild(const Config& config, const std::string& name, SystemTime start_time) override {