This is an automated email from the ASF dual-hosted git repository.

yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new 50deb10372e [fix](be) Keep credentials out of fragment execution logs 
(#69540)
50deb10372e is described below

commit 50deb10372ecdc3f45ac59fc13677ed18eca24bd
Author: Oliveira <[email protected]>
AuthorDate: Mon Oct 26 10:49:26 2026 +0800

    [fix](be) Keep credentials out of fragment execution logs (#69540)
    
    ### What problem does this PR solve?
    
    Problem Summary: FragmentMgr prints complete pipeline execution
    parameters at VLOG(10), including file-scan properties, descriptor
    passwords and transaction tokens. The external open path also prints a
    complete query plan at VLOG(1). Replace these dumps with query/fragment
    identifiers and instance counts without modifying execution parameters.
    
    ```diff
    - Log complete execution parameters and external query plans
    + Log fixed identifiers and instance counts
    ```
    
    ### Release note
    
    Verbose fragment execution logs no longer include full execution
    parameters, query options or external query plans that may contain
    credentials.
    
    ### Evidence
    
    - `FragmentParamsLogTest.*:FragmentMgrLogTest.*`: **5 executed, 5
    passed, 0 failed, 0 skipped** (Clang21/ASAN with the documented private
    legacy compatibility overlay). Fresh GTest XML and unchanged source
    inputs were checked.
    - Tests cover synthetic S3/token/password values, unknown property keys,
    unchanged execution parameters, and real pipeline/external request log
    capture with VLOG(10) enabled.
    - clang-format 16 and whitespace checks passed.
    - ASAN FE+BE build, GLIBC_2.17 artifact check, build hygiene and
    clang-tidy passed.
    - An isolated 1FE+3BE instance scanned a private MinIO CSV: 3 rows and
    SUM(id)=6, independently checked against the uploaded object. Native
    regression output generation and subsequent snapshot comparison both
    passed (1 suite each, no failures/skips).
    - All four real S3 query IDs matched the VLOG(10) fragment summaries;
    synthetic access/secret keys were absent from BE logs. Process
    identities remained unchanged during the test. The task-owned services
    were stopped afterwards.
    
    ### Check List (For Author)
    
    - Test:
        - [x] Unit Test: the five focused fragment log tests above.
    - [x] Manual test: isolated S3 CSV scan, native snapshot comparison, and
    credential-log inspection.
    - Behavior changed:
    - [x] Yes. Verbose diagnostics contain fixed identifiers and counts
    instead of full parameter dumps.
    - Does this need documentation?
        - [x] No.
    
    ### Merge Danger
    
    **Door:** two-way
    
    **Blast Radius:** diagnostics
    
    Execution and protocol parameters are unchanged; only verbose log
    contents change.
    
    ### Check List (For Reviewer who merge this PR)
    
    - [ ] Confirm the release note
    - [ ] Confirm test cases
    - [ ] Confirm document
    - [ ] Add branch pick label
---
 be/src/runtime/fragment_mgr.cpp           |  15 ++--
 be/src/util/debug_util.cpp                |   6 ++
 be/src/util/debug_util.h                  |   5 ++
 be/test/runtime/fragment_mgr_log_test.cpp | 143 ++++++++++++++++++++++++++++++
 be/test/util/fragment_params_log_test.cpp |  69 ++++++++++++++
 5 files changed, 228 insertions(+), 10 deletions(-)

diff --git a/be/src/runtime/fragment_mgr.cpp b/be/src/runtime/fragment_mgr.cpp
index 23666c7258b..ac3684c12f5 100644
--- a/be/src/runtime/fragment_mgr.cpp
+++ b/be/src/runtime/fragment_mgr.cpp
@@ -651,12 +651,7 @@ Status FragmentMgr::exec_plan_fragment(const 
TPipelineFragmentParams& params,
                                        QuerySource query_source, const 
FinishCallback& cb,
                                        const TPipelineFragmentParamsList& 
parent,
                                        std::shared_ptr<bool> 
is_prepare_success) {
-    VLOG_ROW << "Query: " << print_id(params.query_id) << " exec_plan_fragment 
params is "
-             << apache::thrift::ThriftDebugString(params).c_str();
-    // sometimes TPipelineFragmentParams debug string is too long and glog
-    // will truncate the log line, so print query options seperately for 
debuggin purpose
-    VLOG_ROW << "Query: " << print_id(params.query_id) << "query options is "
-             << 
apache::thrift::ThriftDebugString(params.query_options).c_str();
+    VLOG_ROW << "exec_plan_fragment " << print_fragment_params_summary(params);
 
     std::shared_ptr<QueryContext> query_ctx;
     RETURN_IF_ERROR(_get_or_create_query_ctx(params, parent, query_source, 
query_ctx));
@@ -1074,8 +1069,8 @@ Status FragmentMgr::exec_external_plan_fragment(const 
TScanOpenParams& params,
     
RETURN_IF_ERROR(_build_external_scan_selected_columns(t_query_plan_info.plan_fragment,
                                                           *desc_tbl, 
selected_columns));
 
-    VLOG_QUERY << "BackendService execute open()  TQueryPlanInfo: "
-               << apache::thrift::ThriftDebugString(t_query_plan_info);
+    VLOG_QUERY << "BackendService execute open() query_id=" << 
print_id(query_id)
+               << ", fragment_instance_id=" << print_id(fragment_instance_id);
     // assign the param used to execute PlanFragment
     TPipelineFragmentParams exec_fragment_params;
     exec_fragment_params.protocol_version = 
(PaloInternalServiceVersion::type)0;
@@ -1131,8 +1126,8 @@ Status FragmentMgr::exec_external_plan_fragment(const 
TScanOpenParams& params,
     query_options.query_type = TQueryType::EXTERNAL;
     query_options.be_exec_version = BeExecVersionManager::get_newest_version();
     exec_fragment_params.__set_query_options(query_options);
-    VLOG_ROW << "external exec_plan_fragment params is "
-             << 
apache::thrift::ThriftDebugString(exec_fragment_params).c_str();
+    VLOG_ROW << "external exec_plan_fragment "
+             << print_fragment_params_summary(exec_fragment_params);
 
     TPipelineFragmentParamsList mocked;
     return exec_plan_fragment(exec_fragment_params, 
QuerySource::EXTERNAL_CONNECTOR, mocked);
diff --git a/be/src/util/debug_util.cpp b/be/src/util/debug_util.cpp
index dd1ee2c7dc0..57ef59b91fc 100644
--- a/be/src/util/debug_util.cpp
+++ b/be/src/util/debug_util.cpp
@@ -20,6 +20,7 @@
 #include <bvar/bvar.h>
 #include <bvar/multi_dimension.h>
 #include <gen_cpp/HeartbeatService_types.h>
+#include <gen_cpp/PaloInternalService_types.h>
 #include <gen_cpp/PlanNodes_types.h>
 #include <gflags/gflags.h>
 #include <stdint.h>
@@ -41,6 +42,11 @@
 
 namespace doris {
 
+std::string print_fragment_params_summary(const TPipelineFragmentParams& 
params) {
+    return fmt::format("query_id={}, fragment_id={}, local_instances={}", 
print_id(params.query_id),
+                       params.fragment_id, params.local_params.size());
+}
+
 std::string print_plan_node_type(const TPlanNodeType::type& type) {
     std::map<int, const char*>::const_iterator i;
     i = _TPlanNodeType_VALUES_TO_NAMES.find(type);
diff --git a/be/src/util/debug_util.h b/be/src/util/debug_util.h
index 5fad41ec637..81e4bcb0c29 100644
--- a/be/src/util/debug_util.h
+++ b/be/src/util/debug_util.h
@@ -26,6 +26,11 @@
 
 namespace doris {
 
+class TPipelineFragmentParams;
+
+// Only format fixed identifiers and counts. Scan properties and descriptors 
may contain credentials.
+std::string print_fragment_params_summary(const TPipelineFragmentParams& 
params);
+
 std::string print_plan_node_type(const TPlanNodeType::type& type);
 std::string print_query_state(const QueryState::type& type);
 std::string PrintTUnit(const TUnit::type& type);
diff --git a/be/test/runtime/fragment_mgr_log_test.cpp 
b/be/test/runtime/fragment_mgr_log_test.cpp
new file mode 100644
index 00000000000..3fae2a5bd03
--- /dev/null
+++ b/be/test/runtime/fragment_mgr_log_test.cpp
@@ -0,0 +1,143 @@
+// 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.
+
+#include <gen_cpp/DorisExternalService_types.h>
+#include <gen_cpp/PaloInternalService_types.h>
+#include <gtest/gtest.h>
+
+#include <memory>
+#include <mutex>
+#include <string>
+#include <vector>
+
+#include "common/config.h"
+#include "common/logging.h"
+#include "runtime/exec_env.h"
+#include "runtime/fragment_mgr.h"
+#include "util/debug_points.h"
+
+namespace doris {
+
+class FragmentMgrLogTest : public testing::Test, public google::LogSink {
+public:
+    void SetUp() override {
+        _old_debug_points = config::enable_debug_points;
+        config::enable_debug_points = true;
+        // Stop after request logging, before creating a query context or 
dispatching any work.
+        DebugPoints::instance()->add("FragmentMgr._get_query_ctx.failed");
+        _old_vlog_level = google::SetVLOGLevel("fragment_mgr", 10);
+        google::AddLogSink(this);
+        _fragment_mgr = std::make_unique<FragmentMgr>(&_exec_env);
+    }
+
+    void TearDown() override {
+        _fragment_mgr->stop();
+        _fragment_mgr.reset();
+        google::RemoveLogSink(this);
+        google::SetVLOGLevel("fragment_mgr", _old_vlog_level);
+        DebugPoints::instance()->remove("FragmentMgr._get_query_ctx.failed");
+        config::enable_debug_points = _old_debug_points;
+    }
+
+    void send(google::LogSeverity /*severity*/, const char* /*full_filename*/,
+              const char* /*base_filename*/, int /*line*/, const 
google::LogMessageTime& /*time*/,
+              const char* message, std::size_t message_len) override {
+        std::lock_guard lock(_mutex);
+        _messages.append(message, message_len);
+        _messages.push_back('\n');
+    }
+
+protected:
+    static TDescriptorTable credential_descriptors() {
+        TJdbcTable jdbc_table;
+        jdbc_table.__set_jdbc_password("synthetic-jdbc-password");
+        TTableDescriptor table;
+        table.__set_id(1);
+        table.__set_tableType(TTableType::JDBC_TABLE);
+        table.__set_jdbcTable(jdbc_table);
+        TTupleDescriptor tuple;
+        tuple.__set_id(0);
+        tuple.__set_byteSize(0);
+        tuple.__set_numNullBytes(0);
+        TDescriptorTable descriptors;
+        descriptors.__set_tupleDescriptors({tuple});
+        descriptors.__set_tableDescriptors({table});
+        return descriptors;
+    }
+
+    std::string messages() {
+        std::lock_guard lock(_mutex);
+        return _messages;
+    }
+
+    ExecEnv _exec_env;
+    std::unique_ptr<FragmentMgr> _fragment_mgr;
+
+private:
+    std::mutex _mutex;
+    std::string _messages;
+    bool _old_debug_points = false;
+    int _old_vlog_level = 0;
+};
+
+TEST_F(FragmentMgrLogTest, PipelineRequestOmitsCredentials) {
+    TPipelineFragmentParams params;
+    TUniqueId query_id;
+    query_id.__set_hi(0x1234);
+    query_id.__set_lo(0x5678);
+    params.__set_query_id(query_id);
+    params.__set_fragment_id(9);
+    params.__set_desc_tbl(credential_descriptors());
+    TFileScanRangeParams scan_params;
+    scan_params.__set_properties({{"AWS_SECRET_KEY", 
"synthetic-storage-secret"},
+                                  {"unknown_property", 
"synthetic-future-credential"}});
+    params.__set_file_scan_params({{1, scan_params}});
+    TTxnParams txn_params;
+    txn_params.__set_token("synthetic-transaction-token");
+    params.__set_txn_conf(txn_params);
+
+    const auto status = _fragment_mgr->exec_plan_fragment(params, 
QuerySource::INTERNAL_FRONTEND,
+                                                          
TPipelineFragmentParamsList {});
+    EXPECT_FALSE(status.ok());
+    EXPECT_NE(std::string::npos, 
status.to_string().find("FragmentMgr._get_query_ctx.failed"));
+    const auto log = messages();
+    EXPECT_NE(std::string::npos, log.find("exec_plan_fragment 
query_id=1234-5678, fragment_id=9"));
+    EXPECT_EQ(std::string::npos, log.find("synthetic-"));
+}
+
+TEST_F(FragmentMgrLogTest, ExternalRequestOmitsDescriptorCredentials) {
+    TQueryPlanInfo query_plan;
+    query_plan.__set_desc_tbl(credential_descriptors());
+    TUniqueId query_id;
+    query_id.__set_hi(0x1234);
+    query_id.__set_lo(0x5678);
+    TUniqueId instance_id;
+    instance_id.__set_hi(0xabcd);
+    instance_id.__set_lo(0xef01);
+    std::vector<TScanColumnDesc> selected_columns;
+
+    const auto status = _fragment_mgr->exec_external_plan_fragment(
+            TScanOpenParams {}, query_plan, query_id, instance_id, 
&selected_columns);
+    EXPECT_FALSE(status.ok());
+    EXPECT_NE(std::string::npos, 
status.to_string().find("FragmentMgr._get_query_ctx.failed"));
+    const auto log = messages();
+    EXPECT_NE(std::string::npos, log.find("query_id=1234-5678, 
fragment_instance_id=abcd-ef01"));
+    EXPECT_NE(std::string::npos, log.find("external exec_plan_fragment 
query_id=1234-5678"));
+    EXPECT_EQ(std::string::npos, log.find("synthetic-"));
+}
+
+} // namespace doris
diff --git a/be/test/util/fragment_params_log_test.cpp 
b/be/test/util/fragment_params_log_test.cpp
new file mode 100644
index 00000000000..833996c4740
--- /dev/null
+++ b/be/test/util/fragment_params_log_test.cpp
@@ -0,0 +1,69 @@
+// 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.
+
+#include <gen_cpp/PaloInternalService_types.h>
+#include <gtest/gtest.h>
+
+#include "util/debug_util.h"
+
+namespace doris {
+
+TEST(FragmentParamsLogTest, KeepsIdentifiersAndInstanceCount) {
+    TPipelineFragmentParams params;
+    TUniqueId query_id;
+    query_id.__set_hi(0x1234);
+    query_id.__set_lo(0x5678);
+    params.__set_query_id(query_id);
+    params.__set_fragment_id(9);
+    params.local_params.resize(2);
+
+    EXPECT_EQ("query_id=1234-5678, fragment_id=9, local_instances=2",
+              print_fragment_params_summary(params));
+}
+
+TEST(FragmentParamsLogTest, OmitsCredentialsWithoutChangingExecutionParams) {
+    TPipelineFragmentParams params;
+    TFileScanRangeParams scan_params;
+    scan_params.__set_properties({{"AWS_ACCESS_KEY", "synthetic-access-key"},
+                                  {"AWS_SECRET_KEY", "synthetic-secret-key"},
+                                  {"AWS_TOKEN", "synthetic-session-token"},
+                                  {"unknown_property", 
"synthetic-future-credential"}});
+    params.__set_file_scan_params({{1, scan_params}});
+
+    TJdbcTable jdbc_table;
+    jdbc_table.__set_jdbc_password("synthetic-jdbc-password");
+    TTableDescriptor table;
+    table.__set_jdbcTable(jdbc_table);
+    table.__set_tableName("synthetic-sensitive-table-name");
+    TDescriptorTable descriptors;
+    descriptors.__set_tableDescriptors({table});
+    params.__set_desc_tbl(descriptors);
+    const auto original_params = params;
+
+    // Neither known credential keys nor future free-form properties belong in 
the summary.
+    EXPECT_EQ("query_id=0-0, fragment_id=0, local_instances=0",
+              print_fragment_params_summary(params));
+    EXPECT_EQ(original_params, params);
+}
+
+TEST(FragmentParamsLogTest, HandlesAbsentOptionalParams) {
+    const TPipelineFragmentParams params;
+    EXPECT_EQ("query_id=0-0, fragment_id=0, local_instances=0",
+              print_fragment_params_summary(params));
+}
+
+} // namespace doris


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to