diff --git a/src/bvar/default_variables.cpp b/src/bvar/default_variables.cpp index db74db11f5..ecce177717 100644 --- a/src/bvar/default_variables.cpp +++ b/src/bvar/default_variables.cpp @@ -20,6 +20,7 @@ #include // getpagesize #include #include // getrusage +#include // uname #include // dirent #include // setw #include @@ -39,6 +40,7 @@ #include "butil/process_util.h" // ReadCommandLine #include "butil/popen.h" // read_command_output #include "bvar/passive_status.h" +#include "bvar/default_variables.h" // make_kernel_version_string namespace bvar { @@ -617,12 +619,14 @@ static void get_cmdline(std::ostream& os, void*) { struct ReadVersion { std::string content; ReadVersion() { - std::ostringstream oss; - if (butil::read_command_output(oss, "uname -ap") != 0) { - LOG(ERROR) << "Fail to read kernel version"; + struct utsname buf; + if (uname(&buf) != 0) { + const int saved_errno = errno; + LOG(ERROR) << "Failed to read kernel version, errno=" << saved_errno + << " (" << berror(saved_errno) << ")"; return; } - content.append(oss.str()); + content.append(make_kernel_version_string(buf)); } }; static void get_kernel_version(std::ostream& os, void*) { diff --git a/src/bvar/default_variables.h b/src/bvar/default_variables.h new file mode 100644 index 0000000000..8d7e474aee --- /dev/null +++ b/src/bvar/default_variables.h @@ -0,0 +1,64 @@ +// 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. + +#ifndef BVAR_DEFAULT_VARIABLES_H +#define BVAR_DEFAULT_VARIABLES_H + +#include // struct utsname +#include // std::ostringstream +#include // std::string + +namespace bvar { + +// Build the value of the `kernel_version` bvar from a uname(2) result. +// The field layout matches `uname -ap` on Linux and macOS: +// Linux : sysname nodename release version machine processor machine GNU/Linux +// macOS : sysname nodename release version machine processor +// +// uname(2) exposes no separate processor (-p) or hardware-platform (-i) +// field, so both fall back to `machine`. That matches `uname -ap` on the +// platforms brpc targets (Linux/macOS); treat it as best-effort elsewhere. +// +// This is intentionally a header-only helper so that it is shared by both +// default_variables.cpp and the unit tests. default_variables.o is stripped +// from unit-test binaries (see BVAR_NOT_LINK_DEFAULT_VARIABLES in +// variable.cpp), so keeping the formatting logic here lets tests exercise the +// exact production formatter without depending on that object being linked. +inline std::string make_kernel_version_string(const struct utsname& buf) { +#if defined(__APPLE__) && (defined(__aarch64__) || defined(__arm64__)) + const char* processor = "arm"; +#elif defined(__APPLE__) && defined(__x86_64__) + const char* processor = "i386"; +#else + const char* processor = buf.machine; +#endif + std::ostringstream oss; + oss << buf.sysname << ' ' << buf.nodename << ' ' + << buf.release << ' ' << buf.version << ' ' + << buf.machine << ' ' << processor; +#if defined(__linux__) + // `uname -a` appends the hardware platform and the operating-system + // identifier on Linux; the hardware platform equals `machine` here. + oss << ' ' << buf.machine << " GNU/Linux"; +#endif + oss << '\n'; + return oss.str(); +} + +} // namespace bvar + +#endif // BVAR_DEFAULT_VARIABLES_H diff --git a/test/bvar_variable_unittest.cpp b/test/bvar_variable_unittest.cpp index f2a3edb777..0d14fbd7b6 100644 --- a/test/bvar_variable_unittest.cpp +++ b/test/bvar_variable_unittest.cpp @@ -19,7 +19,8 @@ #include // pthread_* #include // usleep - +#include // uname +#include // strlen #include #include #include @@ -29,6 +30,7 @@ #include "butil/macros.h" #include "bvar/bvar.h" +#include "bvar/default_variables.h" // make_kernel_version_string #include #include @@ -462,6 +464,48 @@ TEST_F(VariableTest, dtor_waits_for_inflight_describe) { ASSERT_TRUE(destructed.load()); } + +TEST_F(VariableTest, kernel_version_contains_uname_fields) { + struct utsname buf; + ASSERT_EQ(0, uname(&buf)); + + // Each field should be non-empty + ASSERT_GT(std::strlen(buf.sysname), 0u); + ASSERT_GT(std::strlen(buf.nodename), 0u); + ASSERT_GT(std::strlen(buf.release), 0u); + ASSERT_GT(std::strlen(buf.version), 0u); + ASSERT_GT(std::strlen(buf.machine), 0u); + + // Exercise the exact formatter that backs the kernel_version bvar. It is a + // header-only helper shared with default_variables.cpp, so this validates + // the real production formatting without depending on default_variables.o + // being linked into the unit-test binary: that object is stripped via + // BVAR_NOT_LINK_DEFAULT_VARIABLES, so the bvar is not registered here and + // describe_exposed("kernel_version") would return nothing. + const std::string content = bvar::make_kernel_version_string(buf); + ASSERT_FALSE(content.empty()); + + // The formatted value should contain all the key uname fields. + ASSERT_NE(content.find(buf.sysname), std::string::npos); + ASSERT_NE(content.find(buf.nodename), std::string::npos); + ASSERT_NE(content.find(buf.release), std::string::npos); + ASSERT_NE(content.find(buf.version), std::string::npos); + ASSERT_NE(content.find(buf.machine), std::string::npos); + + // The trailing newline must be preserved to match the previous + // popen("uname -ap") output that this bvar used to expose. + ASSERT_EQ('\n', content[content.size() - 1]); + + // On Linux, sysname is "Linux" and the OS suffix is appended; on macOS, + // sysname is "Darwin" and there is no OS suffix (both match `uname -ap`). +#if defined(__linux__) + ASSERT_STREQ(buf.sysname, "Linux"); + ASSERT_NE(content.find("GNU/Linux"), std::string::npos); +#elif defined(__APPLE__) + ASSERT_STREQ(buf.sysname, "Darwin"); + ASSERT_EQ(content.find("GNU/Linux"), std::string::npos); +#endif +} } // namespace int main(int argc, char** argv) {