From 45454b5c31e195efeac0c380c7c58f2083008537 Mon Sep 17 00:00:00 2001 From: zirain Date: Thu, 16 May 2024 15:36:24 +0800 Subject: [PATCH 1/5] add %TRACE_ID% formatter Signed-off-by: zirain --- .../common/formatter/http_specific_formatter.cc | 15 +++++++++++++++ source/common/formatter/http_specific_formatter.h | 13 +++++++++++++ 2 files changed, 28 insertions(+) diff --git a/source/common/formatter/http_specific_formatter.cc b/source/common/formatter/http_specific_formatter.cc index 4ff9dd4fceae..2ded74047818 100644 --- a/source/common/formatter/http_specific_formatter.cc +++ b/source/common/formatter/http_specific_formatter.cc @@ -197,6 +197,17 @@ HeadersByteSizeFormatter::formatValueWithContext(const HttpFormatterContext& con context.requestHeaders(), context.responseHeaders(), context.responseTrailers())); } +ProtobufWkt::Value TraceIDFormatter::formatValueWithContext(const HttpFormatterContext& context, + const StreamInfo::StreamInfo&) const { + return ValueUtil::stringValue(context.activeSpan().getTraceIdAsHex()); +} + +absl::optional +TraceIDFormatter::formatWithContext(const HttpFormatterContext& context, + const StreamInfo::StreamInfo&) const { + return context.activeSpan().getTraceIdAsHex(); +} + GrpcStatusFormatter::Format GrpcStatusFormatter::parseFormat(absl::string_view format) { if (format.empty() || format == "CAMEL_STRING") { return GrpcStatusFormatter::CamelString; @@ -390,6 +401,10 @@ HttpBuiltInCommandParser::getKnownFormatters() { return std::make_unique(main_header, alternative_header, max_length); + }}}, + {"TRACE_ID", + {CommandSyntaxChecker::COMMAND_ONLY, [](const std::string&, absl::optional&) { + return std::make_unique(); }}}}); } diff --git a/source/common/formatter/http_specific_formatter.h b/source/common/formatter/http_specific_formatter.h index 106355aeb449..890322ff0c04 100644 --- a/source/common/formatter/http_specific_formatter.h +++ b/source/common/formatter/http_specific_formatter.h @@ -143,6 +143,19 @@ class ResponseTrailerFormatter : public FormatterProvider, HeaderFormatter { const StreamInfo::StreamInfo& stream_info) const override; }; +/** + * FormatterProvider for trace ID. + */ +class TraceIDFormatter : public FormatterProvider { +public: + absl::optional + formatWithContext(const HttpFormatterContext& context, + const StreamInfo::StreamInfo& stream_info) const override; + ProtobufWkt::Value + formatValueWithContext(const HttpFormatterContext& context, + const StreamInfo::StreamInfo& stream_info) const override; +}; + class GrpcStatusFormatter : public FormatterProvider, HeaderFormatter { public: enum Format { From f873f34dbfdbefa1cae8a17332c4b3cebd8d9a92 Mon Sep 17 00:00:00 2001 From: zirain Date: Thu, 16 May 2024 16:17:41 +0800 Subject: [PATCH 2/5] doc and test Signed-off-by: zirain --- .../observability/access_log/usage.rst | 6 +++++ .../formatter/substitution_formatter_test.cc | 24 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/docs/root/configuration/observability/access_log/usage.rst b/docs/root/configuration/observability/access_log/usage.rst index 7472c046e2da..b2127d40e1da 100644 --- a/docs/root/configuration/observability/access_log/usage.rst +++ b/docs/root/configuration/observability/access_log/usage.rst @@ -1210,3 +1210,9 @@ UDP %ENVIRONMENT(X):Z% Environment value of environment variable X. If no valid environment variable X, '-' symbol will be used. Z is an optional parameter denoting string truncation up to Z characters long. + +%TRACE_ID% + HTTP + The trace ID of the request. If the request does not have a trace ID, this will be an empty string. + TCP/UDP + Not implemented ("-"). diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index b24cec5eae96..9302d5188c12 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -25,6 +25,7 @@ #include "test/mocks/network/mocks.h" #include "test/mocks/ssl/mocks.h" #include "test/mocks/stream_info/mocks.h" +#include "test/mocks/tracing/mocks.h" #include "test/mocks/upstream/cluster_info.h" #include "test/test_common/environment.h" #include "test/test_common/printers.h" @@ -2144,6 +2145,29 @@ TEST(SubstitutionFormatterTest, responseTrailerFormatter) { } } +TEST(SubstitutionFormatterTest, Trac) { + StreamInfo::MockStreamInfo stream_info; + Http::TestRequestHeaderMapImpl request_header{}; + Http::TestResponseHeaderMapImpl response_header{}; + Http::TestResponseTrailerMapImpl response_trailer{}; + std::string body; + + Tracing::MockSpan active_span; + EXPECT_CALL(active_span, getTraceIdAsHex()) + .WillRepeatedly(Return("ae0046f9075194306d7de2931bd38ce3")); + + HttpFormatterContext formatter_context(&request_header, &response_header, &response_trailer, body, + AccessLogType::NotSet, &active_span); + + { + TraceIDFormatter formatter{}; + EXPECT_EQ("ae0046f9075194306d7de2931bd38ce3", + formatter.formatWithContext(formatter_context, stream_info)); + EXPECT_THAT(formatter.formatValueWithContext(formatter_context, stream_info), + ProtoEq(ValueUtil::stringValue("ae0046f9075194306d7de2931bd38ce3"))); + } +} + /** * Populate a metadata object with the following test data: * "com.test": {"test_key":"test_value","test_obj":{"inner_key":"inner_value"}} From b90454691301aa68cc660c6b96cd5240984a45ed Mon Sep 17 00:00:00 2001 From: zirain Date: Thu, 16 May 2024 16:19:21 +0800 Subject: [PATCH 3/5] changelog Signed-off-by: zirain --- changelogs/current.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 2a4d1c464585..2350d45680e0 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -163,6 +163,9 @@ new_features: change: | added support for :ref:`%UPSTREAM_HOST_NAME% ` for the upstream host identifier. +- area: access_loggers + change: | + Added ``TRACE_ID`` :ref:`access log formatter `. - area: healthcheck change: | Added support to healthcheck with ProxyProtocol in TCP Healthcheck by setting From bfdb72a85c669160d63997fb42a83e75f5e37353 Mon Sep 17 00:00:00 2001 From: zirain Date: Fri, 17 May 2024 04:20:52 +0800 Subject: [PATCH 4/5] nit Signed-off-by: zirain --- test/common/formatter/substitution_formatter_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index 9302d5188c12..5ce4377d35b8 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -2145,7 +2145,7 @@ TEST(SubstitutionFormatterTest, responseTrailerFormatter) { } } -TEST(SubstitutionFormatterTest, Trac) { +TEST(SubstitutionFormatterTest, TraceIDFormatter) { StreamInfo::MockStreamInfo stream_info; Http::TestRequestHeaderMapImpl request_header{}; Http::TestResponseHeaderMapImpl response_header{}; From 46777ac5b328f6c68c35595668fe5545fd50b067 Mon Sep 17 00:00:00 2001 From: zirain Date: Fri, 17 May 2024 04:38:28 +0800 Subject: [PATCH 5/5] unspecifiedValue Signed-off-by: zirain --- source/common/formatter/http_specific_formatter.cc | 12 ++++++++++-- .../formatter/substitution_formatter_test.cc | 14 +++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/source/common/formatter/http_specific_formatter.cc b/source/common/formatter/http_specific_formatter.cc index 2ded74047818..7623c4b95ac9 100644 --- a/source/common/formatter/http_specific_formatter.cc +++ b/source/common/formatter/http_specific_formatter.cc @@ -199,13 +199,21 @@ HeadersByteSizeFormatter::formatValueWithContext(const HttpFormatterContext& con ProtobufWkt::Value TraceIDFormatter::formatValueWithContext(const HttpFormatterContext& context, const StreamInfo::StreamInfo&) const { - return ValueUtil::stringValue(context.activeSpan().getTraceIdAsHex()); + auto trace_id = context.activeSpan().getTraceIdAsHex(); + if (trace_id.empty()) { + return SubstitutionFormatUtils::unspecifiedValue(); + } + return ValueUtil::stringValue(trace_id); } absl::optional TraceIDFormatter::formatWithContext(const HttpFormatterContext& context, const StreamInfo::StreamInfo&) const { - return context.activeSpan().getTraceIdAsHex(); + auto trace_id = context.activeSpan().getTraceIdAsHex(); + if (trace_id.empty()) { + return absl::nullopt; + } + return trace_id; } GrpcStatusFormatter::Format GrpcStatusFormatter::parseFormat(absl::string_view format) { diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index 5ce4377d35b8..ef0b1704d9b4 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -2156,16 +2156,24 @@ TEST(SubstitutionFormatterTest, TraceIDFormatter) { EXPECT_CALL(active_span, getTraceIdAsHex()) .WillRepeatedly(Return("ae0046f9075194306d7de2931bd38ce3")); - HttpFormatterContext formatter_context(&request_header, &response_header, &response_trailer, body, - AccessLogType::NotSet, &active_span); - { + HttpFormatterContext formatter_context(&request_header, &response_header, &response_trailer, + body, AccessLogType::NotSet, &active_span); TraceIDFormatter formatter{}; EXPECT_EQ("ae0046f9075194306d7de2931bd38ce3", formatter.formatWithContext(formatter_context, stream_info)); EXPECT_THAT(formatter.formatValueWithContext(formatter_context, stream_info), ProtoEq(ValueUtil::stringValue("ae0046f9075194306d7de2931bd38ce3"))); } + + { + HttpFormatterContext formatter_context(&request_header, &response_header, &response_trailer, + body); + TraceIDFormatter formatter{}; + EXPECT_EQ(absl::nullopt, formatter.formatWithContext(formatter_context, stream_info)); + EXPECT_THAT(formatter.formatValueWithContext(formatter_context, stream_info), + ProtoEq(ValueUtil::nullValue())); + } } /**