rename getTraceIdAsHex to getTraceId (#34314)

* rename getTraceId Signed-off-by: zirain <zirain2009@gmail.com> * implement getTraceId Signed-off-by: zirain <zirain2009@gmail.com> * fix test Signed-off-by: zirain <zirain2009@gmail.com> * fix test Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com>
envoyproxy · May 27, 2024 · d19462e · d19462e
1 parent d69544e
commit d19462e
Show file tree

Hide file tree

Showing 25 changed files with 39 additions and 40 deletions.
diff --git a/envoy/tracing/trace_driver.h b/envoy/tracing/trace_driver.h
@@ -15,7 +15,7 @@ class Span;
 using SpanPtr = std::unique_ptr<Span>;
 
 /**
- * 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
@@ -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<std::string>
 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
@@ -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
@@ -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
@@ -46,7 +46,9 @@ class OpenTracingSpan : public Tracing::Span, Logger::Loggable<Logger::Id::traci
   void setBaggage(absl::string_view key, absl::string_view value) override;
 
   // TODO: This method is unimplemented for OpenTracing.
-  std::string getTraceIdAsHex() const override { return EMPTY_STRING; };
+  // This won't be implemented because OpenTracing was deprecated.
+  // We should remove this method in the future?
+  std::string getTraceId() const override { return EMPTY_STRING; };
 
 private:
   OpenTracingDriver& driver_;

diff --git a/source/extensions/tracers/datadog/span.cc b/source/extensions/tracers/datadog/span.cc
@@ -135,7 +135,7 @@ void Span::setBaggage(absl::string_view, absl::string_view) {
   // not implemented
 }
 
-std::string Span::getTraceIdAsHex() const {
+std::string Span::getTraceId() const {
   if (!span_) {
     return std::string{};
   }

diff --git a/source/extensions/tracers/datadog/span.h b/source/extensions/tracers/datadog/span.h
@@ -47,7 +47,7 @@ class Span : public Tracing::Span {
   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;
+  std::string getTraceId() const override;
 
 private:
   datadog::tracing::Optional<datadog::tracing::Span> span_;

diff --git 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
@@ -39,9 +39,8 @@ void callSampler(SamplerSharedPtr sampler, const absl::optional<SpanContext> 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
@@ -117,7 +117,7 @@ class Span : Logger::Loggable<Logger::Id::tracing>, 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
@@ -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
@@ -231,7 +231,7 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
   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
@@ -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
@@ -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
@@ -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
@@ -175,7 +175,7 @@ TEST_F(AccessLogTest, TraceId) {
 
   NiceMock<Tracing::MockSpan> 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<Tracing::MockSpan> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 {