feat userver: http client/server spans according to OTel semconv

commit_hash:1b0726dd0443095faa71fb0ae7c1e6a408a979f9

Author: miglitskii

chaotic-openapi/chaotic_openapi/back/cpp_client/templates/client_impl.cpp.jinja

@@ -7,7 +7,7 @@
 namespace {{ namespace }} {
 
 ClientImpl::ClientImpl(const USERVER_NAMESPACE::chaotic::openapi::client::Config& config,
-             USERVER_NAMESPACE::clients::http::Client& http_client) 
+             USERVER_NAMESPACE::clients::http::Client& http_client)
     : config_(config), http_client_(http_client)
 {
 }
@@ -21,15 +21,16 @@ ClientImpl::ClientImpl(const USERVER_NAMESPACE::chaotic::openapi::client::Config
           const USERVER_NAMESPACE::chaotic::openapi::client::CommandControl& command_control
       ) {
         auto r = http_client_.CreateRequest();
+        r.SetUrlTemplate("{{ op.path }}");
         if (plugins_) {
             r.SetPluginsList(*plugins_);
         }
         ApplyConfig(r, command_control, config_);
-        
+
         {% if not op.empty_request() -%}
         SerializeRequest(request, config_.base_url, r);
         {% endif %}
-        
+
         middleware_manager_.ProcessRequest(r);
 
         std::shared_ptr<USERVER_NAMESPACE::clients::http::Response> response;
@@ -39,7 +40,7 @@ ClientImpl::ClientImpl(const USERVER_NAMESPACE::chaotic::openapi::client::Config
         } catch (const USERVER_NAMESPACE::clients::http::TimeoutException& e) {
             throw {{ op.cpp_namespace() }}::TimeoutException();
         }
-        
+
         return {{ op.cpp_namespace() }}::ParseResponse(*response);
       }
   {% endif %}

chaotic-openapi/integration_tests/src/requests_test.cpp

@@ -115,7 +115,7 @@ UTEST_F(RequestsQueryLogMode, HideOperation) {
 
     EXPECT_EQ(response->status_code(), 200);
 
-    auto text = GetLogCapture().GetAll().back().GetTag("http.url");
+    auto text = GetLogCapture().GetAll().back().GetTag("url.full");
     EXPECT_TRUE(utils::text::EndsWith(text, "test1/query-log-mode?password=***&secret=***"));
 }
 
@@ -132,8 +132,8 @@ UTEST_F(RequestsQueryLogMode, HideParameter) {
 
     EXPECT_EQ(response->status_code(), 200);
 
-    auto text = GetLogCapture().GetAll().back().GetTag("http.url");
-    EXPECT_TRUE(utils::text::EndsWith(text, "test1/query-log-mode/parameter?password=***&secret=bar")) << text;
+    auto text = GetLogCapture().GetAll().back().GetTag("url.full");
+    EXPECT_TRUE(utils::text::EndsWith(text, "test1/query-log-mode/parameter?password=***&secret=bar"));
 }
 
 }  // namespace

core/functional_tests/basic_chaos/tests-deadline/test_client.py

@@ -96,11 +96,11 @@ async def test_timeout_expired(
     # Client metrics are counted per attempt.
     assert client_metrics.value_at('errors', {'http_error': 'timeout', **VERSION}) == attempts
 
-    logs = capture.select(stopwatch_name='external/localhost')
+    logs = capture.select(stopwatch_name='GET localhost')
     assert len(logs) == 1
     assert logs[0]['error'] == '1'
-    assert logs[0]['attempts'] == str(attempts)
-    assert logs[0]['max_attempts'] == str(attempts)
+    assert logs[0]['http.request.resend_count'] == str(attempts)
+    assert logs[0]['http.request.max_resend_count'] == str(attempts)
     assert logs[0].get('cancelled_by_deadline', '0') == '0'
     assert logs[0]['error_msg'] == 'Timeout was reached'
     assert logs[0]['timeout_ms'] == str(timeout)
@@ -163,10 +163,10 @@ async def test_deadline_expired(
         # Client metrics are counted per attempt.
         assert timed_out == attempts
 
-    logs = capture.select(stopwatch_name='external/localhost')
+    logs = capture.select(stopwatch_name='GET localhost')
     assert len(logs) == 1
     assert logs[0]['error'] == '1'
-    assert logs[0]['max_attempts'] == str(attempts)
+    assert logs[0]['http.request.max_resend_count'] == str(attempts)
     assert logs[0]['cancelled_by_deadline'] == '1'
     assert logs[0]['error_msg'] == 'Timeout was reached'
     assert logs[0]['timeout_ms'] == str(timeout)
@@ -222,7 +222,7 @@ async def test_fake_deadline_expired_with_exception(
     )
 
     assert deadline_warning['body'] == 'Deadline expired'
-    assert deadline_warning['meta_code'] == '504'
+    assert deadline_warning['http.response.status_code'] == '504'
 
 
 async def test_fake_deadline_expired(
@@ -247,11 +247,11 @@ async def test_fake_deadline_expired(
     assert client_metrics.value_at('errors', {'http_error': 'ok', **VERSION}) == 0
     assert client_metrics.value_at('errors', {'http_error': 'timeout', **VERSION}) == 1
 
-    logs = capture.select(stopwatch_name='external/localhost')
+    logs = capture.select(stopwatch_name='GET localhost')
     assert len(logs) == 1
     assert logs[0]['error'] == '1'
-    assert logs[0]['attempts'] == '1'
-    assert logs[0]['max_attempts'] == '3'
+    assert logs[0]['http.request.resend_count'] == '1'
+    assert logs[0]['http.request.max_resend_count'] == '3'
     assert logs[0]['cancelled_by_deadline'] == '1'
     assert logs[0]['error_msg'] == 'Timeout was reached'
     assert logs[0]['timeout_ms'] == '500'

core/functional_tests/tracing/tests/conftest.py

@@ -72,11 +72,11 @@ async def _check_the_files(check_trace_id: str):
         f'\ttrace_id={trace_id}',
         'service_name=http-tracing-test',
         'duration=',
-        'operation_name=external',
+        'operation_name=GET localhost',
         'tags=[{"',
         'test-service/echo-no-body"',
-        '"key":"http.url"',
-        '"key":"http.status_code"',
+        '"key":"url.full"',
+        '"key":"http.response.status_code"',
         '"value":"200"',
         '}]',
     }

core/include/userver/clients/http/request.hpp

@@ -259,6 +259,11 @@ class Request final {
     Request& SetLoggedUrl(std::string url) &;
     Request SetLoggedUrl(std::string url) &&;
 
+    /// Set URL template (low-cardinality) for tracing, i.e. `/v1/user/{user_id}`
+    /// @see https://opentelemetry.io/docs/specs/semconv/registry/attributes/url/
+    Request& SetUrlTemplate(std::string url_template) &;
+    Request SetUrlTemplate(std::string url_template) &&;
+
     /// Set destination name in metric "httpclient.destinations.<name>".
     /// If not set, defaults to HTTP path.  Should be called for all requests
     /// with parameters in HTTP path.

core/include/userver/tracing/tags.hpp

@@ -21,15 +21,25 @@ extern const std::string kSpanKindServer;
 extern const std::string kSpanKindClient;
 extern const std::string kSpanKindInternal;
 extern const std::string kHttpUrl;
+extern const std::string kUrlFull;
+extern const std::string kHttpUrlTemplate;
 extern const std::string kHttpMetaType;
 extern const std::string kHttpMethod;
+extern const std::string kHttpRequestMethod;
+extern const std::string kHttpRoute;
 extern const std::string kHttpStatusCode;
+extern const std::string kHttpResponseStatusCode;
 extern const std::string kAttempts;
 extern const std::string kMaxAttempts;
 extern const std::string kTimeoutMs;
 extern const std::string kErrorFlag;
 extern const std::string kErrorMessage;
 
+extern const std::string kRpcSystem;
+extern const std::string kRpcService;
+extern const std::string kRpcMethod;
+extern const std::string kGrpcCode;
+
 extern const std::string kDatabaseType;
 extern const std::string kDatabaseMongoType;
 extern const std::string kDatabasePostgresType;
@@ -42,6 +52,7 @@ extern const std::string kDatabaseStatementName;
 extern const std::string kDatabaseStatementDescription;
 
 extern const std::string kPeerAddress;
+extern const std::string kServerAddress;
 
 }  // namespace tracing
 

core/src/clients/http/request.cpp

@@ -34,8 +34,6 @@ namespace clients::http {
 
 namespace {
 
-constexpr std::string_view kHeaderExpect = "Expect";
-
 static constexpr utils::TrivialBiMap kHttpMethodMap([](auto selector) {
     return selector()
         .Case(HttpMethod::kGet, "GET")
@@ -47,8 +45,6 @@ static constexpr utils::TrivialBiMap kHttpMethodMap([](auto selector) {
         .Case(HttpMethod::kOptions, "OPTIONS");
 });
 
-std::string ToString(HttpMethod method) { return std::string{ToStringView(method)}; }
-
 curl::easy::http_version_t ToNative(HttpVersion version) {
     switch (version) {
         case HttpVersion::kDefault:
@@ -340,8 +336,7 @@ Request& Request::connect_to(const ConnectTo& connect_to) & {
 Request Request::connect_to(const ConnectTo& connect_to) && { return std::move(this->connect_to(connect_to)); }
 
 Request& Request::data(std::string data) & {
-    if (!data.empty()) pimpl_->easy().add_header(kHeaderExpect, "", curl::easy::EmptyHeaderAction::kDoNotSend);
-    pimpl_->easy().set_post_fields(std::move(data));
+    pimpl_->data(data);
     return *this;
 }
 Request Request::data(std::string data) && { return std::move(this->data(std::move(data))); }
@@ -427,28 +422,7 @@ Request Request::cookies(const std::unordered_map<std::string, std::string>& coo
 }
 
 Request& Request::method(HttpMethod method) & {
-    switch (method) {
-        case HttpMethod::kDelete:
-        case HttpMethod::kOptions:
-            pimpl_->easy().set_custom_request(ToString(method));
-            break;
-        case HttpMethod::kGet:
-            pimpl_->easy().set_http_get(true);
-            pimpl_->easy().set_custom_request(nullptr);
-            break;
-        case HttpMethod::kHead:
-            pimpl_->easy().set_no_body(true);
-            pimpl_->easy().set_custom_request(nullptr);
-            break;
-        // NOTE: set_post makes libcURL to read from stdin if no data is set
-        case HttpMethod::kPost:
-        case HttpMethod::kPut:
-        case HttpMethod::kPatch:
-            pimpl_->easy().set_custom_request(ToString(method));
-            // ensure a body as we should send Content-Length for this method
-            if (!pimpl_->easy().has_post_data()) data({});
-            break;
-    };
+    pimpl_->SetMethod(method);
     return *this;
 }
 
@@ -536,6 +510,15 @@ Request& Request::SetLoggedUrl(std::string url) & {
 }
 Request Request::SetLoggedUrl(std::string url) && { return std::move(this->SetLoggedUrl(std::move(url))); }
 
+Request& Request::SetUrlTemplate(std::string url_template) & {
+    pimpl_->SetUrlTemplate(std::move(url_template));
+    return *this;
+}
+
+Request Request::SetUrlTemplate(std::string url_template) && {
+    return std::move(this->SetUrlTemplate(std::move(url_template)));
+}
+
 Request& Request::SetDestinationMetricName(const std::string& destination) & {
     pimpl_->SetDestinationMetricName(destination);
     return *this;

core/src/clients/http/request_state.cpp

@@ -19,6 +19,7 @@
 #include <userver/clients/http/connect_to.hpp>
 #include <userver/http/url.hpp>
 #include <userver/server/request/task_inherited_data.hpp>
+#include <userver/utils/algo.hpp>
 #include <userver/utils/assert.hpp>
 #include <userver/utils/async.hpp>
 #include <userver/utils/encoding/hex.hpp>
@@ -53,8 +54,6 @@ constexpr Status kLeastHttpCodeForDeadlineExpired{400};
 
 constexpr Status kFakeHttpErrorCode{599};
 
-const std::string kTracingClientName = "external/";
-
 constexpr utils::TrivialBiMap kTestsuiteActions = [](auto selector) {
     return selector()
         .Case("timeout", curl::errc::EasyErrorCode::kOperationTimedout)
@@ -100,6 +99,8 @@ bool IsSetCookie(std::string_view key) {
 // Not a strict check, but OK for non-header line check
 bool IsHttpStatusLineStart(const char* ptr, size_t size) { return (size > 5 && memcmp(ptr, "HTTP/", 5) == 0); }
 
+std::string ToString(HttpMethod method) { return std::string{ToStringView(method)}; }
+
 char* RfindNotSpace(char* ptr, size_t size) {
     for (char* p = ptr + size - 1; p >= ptr; --p) {
         const char c = *p;
@@ -484,7 +485,7 @@ void RequestState::OnCompleted(std::shared_ptr<RequestState> holder, std::error_
 
         span.AddTag(tracing::kErrorFlag, true);
         span.AddTag(tracing::kErrorMessage, err.message());
-        span.AddTag(tracing::kHttpStatusCode, kFakeHttpErrorCode);
+        span.AddTag(tracing::kHttpResponseStatusCode, kFakeHttpErrorCode);
 
         if (holder->errorbuffer_.front()) {
             LOG_DEBUG() << "cURL error details: " << holder->errorbuffer_.data();
@@ -506,7 +507,7 @@ void RequestState::OnCompleted(std::shared_ptr<RequestState> holder, std::error_
             }};
         std::visit(visitor, holder->data_);
     } else {
-        span.AddTag(tracing::kHttpStatusCode, status_code);
+        span.AddTag(tracing::kHttpResponseStatusCode, status_code);
         holder->response()->SetStatusCode(status_code);
         holder->response()->SetStats(easy.get_local_stats());
 
@@ -652,6 +653,39 @@ void RequestState::SetPluginsList(const std::vector<utils::NotNull<Plugin*>>& pl
 
 void RequestState::SetLoggedUrl(std::string url) { log_url_ = std::move(url); }
 
+void RequestState::SetUrlTemplate(std::string url_template) { url_template_ = std::move(url_template); }
+
+void RequestState::SetMethod(HttpMethod method) {
+    switch (method) {
+        case HttpMethod::kDelete:
+        case HttpMethod::kOptions:
+            easy().set_custom_request(ToString(method));
+            break;
+        case HttpMethod::kGet:
+            easy().set_http_get(true);
+            easy().set_custom_request(nullptr);
+            break;
+        case HttpMethod::kHead:
+            easy().set_no_body(true);
+            easy().set_custom_request(nullptr);
+            break;
+        // NOTE: set_post makes libcURL to read from stdin if no data is set
+        case HttpMethod::kPost:
+        case HttpMethod::kPut:
+        case HttpMethod::kPatch:
+            easy().set_custom_request(ToString(method));
+            // ensure a body as we should send Content-Length for this method
+            if (!easy().has_post_data()) data({});
+            break;
+    };
+    method_ = method;
+}
+
+void RequestState::data(std::string data) {
+    if (!data.empty()) easy().add_header(kHeaderExpect, "", curl::easy::EmptyHeaderAction::kDoNotSend);
+    easy().set_post_fields(std::move(data));
+}
+
 const std::string& RequestState::GetLoggedOriginalUrl() const noexcept {
     // We may want to use original_url if effective_url is not available yet.
     return log_url_ ? *log_url_ : easy().get_original_url();
@@ -992,16 +1026,30 @@ void RequestState::ApplyTestsuiteConfig() {
 void RequestState::StartNewSpan(utils::impl::SourceLocation location) {
     UINVARIANT(!span_storage_, "Attempt to reuse request while the previous one has not finished");
 
-    span_storage_.emplace(
-        kTracingClientName + std::string{USERVER_NAMESPACE::http::ExtractHostname(easy().get_original_url())}, location
-    );
+    std::string span_name;
+    if (url_template_.has_value()) {
+        span_name = utils::StrCat(ToStringView(method_), " ", url_template_.value());
+    } else {
+        span_name = utils::StrCat(
+            ToStringView(method_), " ", USERVER_NAMESPACE::http::ExtractHostname(easy().get_original_url())
+        );
+    }
+    span_storage_.emplace(std::move(span_name), location);
+
     auto& span = span_storage_->Get();
 
     auto request_editable_instance = GetEditableRequestInstance();
 
     tracing_manager_->FillRequestWithTracingContext(span, request_editable_instance);
     plugin_pipeline_.HookCreateSpan(*this, span);
-    span.AddTag(tracing::kHttpUrl, GetLoggedOriginalUrl());
+    span.AddTag(tracing::kUrlFull, GetLoggedOriginalUrl());
+    span.AddTag(
+        tracing::kServerAddress, std::string{USERVER_NAMESPACE::http::ExtractHostname(easy().get_original_url())}
+    );
+    span.AddTag(tracing::kHttpRequestMethod, std::string{ToStringView(method_)});
+    if (url_template_.has_value()) {
+        span.AddTag(tracing::kHttpUrlTemplate, url_template_.value());
+    }
     span.AddTag(tracing::kMaxAttempts, retry_.retries);
 
     // Span is local to a Request, it is not related to current coroutine

core/src/clients/http/request_state.hpp

@@ -12,6 +12,7 @@
 #include <userver/clients/http/error.hpp>
 #include <userver/clients/http/form.hpp>
 #include <userver/clients/http/plugin.hpp>
+#include <userver/clients/http/request.hpp>
 #include <userver/clients/http/response_future.hpp>
 #include <userver/concurrent/queue.hpp>
 #include <userver/crypto/certificate.hpp>
@@ -37,6 +38,8 @@ USERVER_NAMESPACE_BEGIN
 
 namespace clients::http {
 
+constexpr std::string_view kHeaderExpect = "Expect";
+
 class StreamedResponse;
 class ConnectTo;
 
@@ -136,6 +139,9 @@ class RequestState : public std::enable_shared_from_this<RequestState> {
 
     void SetPluginsList(const std::vector<utils::NotNull<Plugin*>>& plugins);
     void SetLoggedUrl(std::string url);
+    void SetUrlTemplate(std::string url_template);
+    void SetMethod(clients::http::HttpMethod method);
+    void data(std::string data);
     void SetEasyTimeout(std::chrono::milliseconds timeout);
 
     void SetTracingManager(const tracing::TracingManagerBase&);
@@ -235,6 +241,9 @@ class RequestState : public std::enable_shared_from_this<RequestState> {
     std::optional<tracing::InPlaceSpan> span_storage_;
     std::optional<std::string> log_url_;
 
+    std::optional<std::string> url_template_;
+    HttpMethod method_{HttpMethod::kGet};
+
     std::atomic<bool> is_cancelled_{false};
     std::array<char, CURL_ERROR_SIZE> errorbuffer_{};