[improve](http) Save the requested url in http execution error #43855

be/src/http/http_client.cpp

@@ -26,6 +26,7 @@
 #include "common/config.h"
 #include "http/http_headers.h"
 #include "http/http_status.h"
+#include "util/security.h"
 #include "util/stack_util.h"
 
 namespace doris {
@@ -198,9 +199,11 @@ Status HttpClient::execute(const std::function<bool(const void* data, size_t len
     _callback = &callback;
     auto code = curl_easy_perform(_curl);
     if (code != CURLE_OK) {
+        std::string url = mask_token(_get_url());
         LOG(WARNING) << "fail to execute HTTP client, errmsg=" << _to_errmsg(code)
-                     << ", trace=" << get_stack_trace();
-        return Status::HttpError(_to_errmsg(code));
+                     << ", trace=" << get_stack_trace() << ", url=" << url;
+        std::string errmsg = fmt::format("{}, url={}", _to_errmsg(code), url);
+        return Status::HttpError(std::move(errmsg));
     }
     return Status::OK();
 }
@@ -268,13 +271,22 @@ Status HttpClient::execute(std::string* response) {
     return execute(callback);
 }
 
-const char* HttpClient::_to_errmsg(CURLcode code) {
+const char* HttpClient::_to_errmsg(CURLcode code) const {
     if (_error_buf[0] == 0) {
         return curl_easy_strerror(code);
     }
     return _error_buf;
 }
 
+const char* HttpClient::_get_url() const {
+    const char* url = nullptr;
+    curl_easy_getinfo(_curl, CURLINFO_EFFECTIVE_URL, &url);
+    if (!url) {
+        url = "<unknown>";
+    }
+    return url;
+}
+
 Status HttpClient::execute_with_retry(int retry_times, int sleep_time,
                                       const std::function<Status(HttpClient*)>& callback) {
     Status status;
@@ -286,7 +298,9 @@ Status HttpClient::execute_with_retry(int retry_times, int sleep_time,
             if (http_status == 200) {
                 return status;
             } else {
-                auto error_msg = fmt::format("http status code is not 200, code={}", http_status);
+                std::string url = mask_token(client._get_url());
+                auto error_msg = fmt::format("http status code is not 200, code={}, url={}",
+                                             http_status, url);
                 LOG(WARNING) << error_msg;
                 return Status::HttpError(error_msg);
             }

be/src/http/http_client.h

@@ -147,7 +147,8 @@ class HttpClient {
     size_t on_response_data(const void* data, size_t length);
 
 private:
-    const char* _to_errmsg(CURLcode code);
+    const char* _to_errmsg(CURLcode code) const;
+    const char* _get_url() const;
 
 private:
     CURL* _curl = nullptr;

be/src/olap/single_replica_compaction.cpp

@@ -37,6 +37,7 @@
 #include "task/engine_clone_task.h"
 #include "util/brpc_client_cache.h"
 #include "util/doris_metrics.h"
+#include "util/security.h"
 #include "util/thrift_rpc_helper.h"
 #include "util/trace.h"
 
@@ -390,7 +391,7 @@ Status SingleReplicaCompaction::_download_files(DataDir* data_dir,
     // then it will try to clone from BE 2, but it will find the file 1 already exist, but file 1 with same
     // name may have different versions.
     VLOG_DEBUG << "single replica compaction begin to download files, remote path="
-               << remote_url_prefix << " local_path=" << local_path;
+               << mask_token(remote_url_prefix) << " local_path=" << local_path;
     RETURN_IF_ERROR(io::global_local_filesystem()->delete_directory(local_path));
     RETURN_IF_ERROR(io::global_local_filesystem()->create_directory(local_path));
 
@@ -448,9 +449,9 @@ Status SingleReplicaCompaction::_download_files(DataDir* data_dir,
 
         std::string local_file_path = local_path + file_name;
 
-        LOG(INFO) << "single replica compaction begin to download file from: " << remote_file_url
-                  << " to: " << local_file_path << ". size(B): " << file_size
-                  << ", timeout(s): " << estimate_timeout;
+        LOG(INFO) << "single replica compaction begin to download file from: "
+                  << mask_token(remote_file_url) << " to: " << local_file_path
+                  << ". size(B): " << file_size << ", timeout(s): " << estimate_timeout;
 
         auto download_cb = [&remote_file_url, estimate_timeout, &local_file_path,
                             file_size](HttpClient* client) {
@@ -462,7 +463,8 @@ Status SingleReplicaCompaction::_download_files(DataDir* data_dir,
             uint64_t local_file_size = std::filesystem::file_size(local_file_path);
             if (local_file_size != file_size) {
                 LOG(WARNING) << "download file length error"
-                             << ", remote_path=" << remote_file_url << ", file_size=" << file_size
+                             << ", remote_path=" << mask_token(remote_file_url)
+                             << ", file_size=" << file_size
                              << ", local_file_size=" << local_file_size;
                 return Status::InternalError("downloaded file size is not equal");
             }

be/src/olap/single_replica_compaction.h

@@ -58,10 +58,10 @@ class SingleReplicaCompaction : public Compaction {
     Status _download_files(DataDir* data_dir, const std::string& remote_url_prefix,
                            const std::string& local_path);
     Status _release_snapshot(const std::string& ip, int port, const std::string& snapshot_path);
-    Status _finish_clone(const string& clone_dir, const Version& version);
+    Status _finish_clone(const std::string& clone_dir, const Version& version);
     CompactionType _compaction_type;
 
     DISALLOW_COPY_AND_ASSIGN(SingleReplicaCompaction);
 };
 
-} // namespace doris
+} // namespace doris

be/src/olap/task/engine_clone_task.cpp

@@ -31,7 +31,6 @@
 #include <memory>
 #include <mutex>
 #include <ostream>
-#include <regex>
 #include <set>
 #include <shared_mutex>
 #include <system_error>
@@ -63,6 +62,7 @@
 #include "runtime/thread_context.h"
 #include "util/defer_op.h"
 #include "util/network_util.h"
+#include "util/security.h"
 #include "util/stopwatch.hpp"
 #include "util/thrift_rpc_helper.h"
 #include "util/trace.h"
@@ -410,7 +410,7 @@ Status EngineCloneTask::_make_and_download_snapshots(DataDir& data_dir,
                     _clone_req.table_id, _clone_req.partition_id, _clone_req.schema_hash);
         } else {
             LOG_WARNING("failed to download snapshot from remote BE")
-                    .tag("url", _mask_token(remote_url_prefix))
+                    .tag("url", mask_token(remote_url_prefix))
                     .error(status);
         }
 
@@ -554,11 +554,11 @@ Status EngineCloneTask::_download_files(DataDir* data_dir, const std::string& re
 
         std::string local_file_path = local_path + "/" + file_name;
 
-        LOG(INFO) << "clone begin to download file from: " << _mask_token(remote_file_url)
+        LOG(INFO) << "clone begin to download file from: " << mask_token(remote_file_url)
                   << " to: " << local_file_path << ". size(B): " << file_size
                   << ", timeout(s): " << estimate_timeout;
 
-        auto download_cb = [this, &remote_file_url, estimate_timeout, &local_file_path,
+        auto download_cb = [&remote_file_url, estimate_timeout, &local_file_path,
                             file_size](HttpClient* client) {
             RETURN_IF_ERROR(client->init(remote_file_url));
             client->set_timeout_ms(estimate_timeout * 1000);
@@ -574,7 +574,7 @@ Status EngineCloneTask::_download_files(DataDir* data_dir, const std::string& re
             }
             if (local_file_size != file_size) {
                 LOG(WARNING) << "download file length error"
-                             << ", remote_path=" << _mask_token(remote_file_url)
+                             << ", remote_path=" << mask_token(remote_file_url)
                              << ", file_size=" << file_size
                              << ", local_file_size=" << local_file_size;
                 return Status::InternalError("downloaded file size is not equal");
@@ -602,7 +602,7 @@ Status EngineCloneTask::_download_files(DataDir* data_dir, const std::string& re
 
 /// This method will only be called if tablet already exist in this BE when doing clone.
 /// This method will do the following things:
-/// 1. Linke all files from CLONE dir to tablet dir if file does not exist in tablet dir
+/// 1. Link all files from CLONE dir to tablet dir if file does not exist in tablet dir
 /// 2. Call _finish_xx_clone() to revise the tablet meta.
 Status EngineCloneTask::_finish_clone(Tablet* tablet, const std::string& clone_dir,
                                       int64_t committed_version, bool is_incremental_clone) {
@@ -867,9 +867,4 @@ Status EngineCloneTask::_finish_full_clone(Tablet* tablet,
     // TODO(plat1ko): write cooldown meta to remote if this replica is cooldown replica
 }
 
-std::string EngineCloneTask::_mask_token(const std::string& str) {
-    std::regex pattern("token=[\\w|-]+");
-    return regex_replace(str, pattern, "token=******");
-}
-
 } // namespace doris

be/src/olap/task/engine_clone_task.h

@@ -85,8 +85,6 @@ class EngineCloneTask : public EngineTask {
 
     Status _release_snapshot(const std::string& ip, int port, const std::string& snapshot_path);
 
-    std::string _mask_token(const std::string& str);
-
 private:
     const TCloneReq& _clone_req;
     vector<TTabletInfo>* _tablet_infos;

be/src/util/security.h

@@ -0,0 +1,35 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <regex>
+#include <string>
+
+namespace doris {
+
+inline std::string mask_token(const std::string& str) {
+    std::regex pattern("token=[\\w|-]+");
+    return std::regex_replace(str, pattern, "token=******");
+}
+
+inline std::string mask_token(const char* str) {
+    std::regex pattern("token=[\\w|-]+");
+    return std::regex_replace(str, pattern, "token=******");
+}
+
+} // namespace doris