Skip to content

build(deps): Align protobuf and related dependencies with the gRPC v1.81.1 bump #516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mc-nv merged 3 commits into from
Jul 23, 2026
Merged

build(deps): Align protobuf and related dependencies with the gRPC v1.81.1 bump #516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f07e2f3
fix: Pass utf8_range_DIR to triton-core build
mc-nv Jul 17, 2026
3df65f8
fix: Adapt model_config_utils to protobuf v33 API
mc-nv Jul 17, 2026
0d23db6
fix: Log auto-completed model config with stable text format
mc-nv Jul 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ if(NOT TRITON_CORE_HEADERS_ONLY)

# Location where protobuf-config.cmake will be installed
set(_FINDPACKAGE_PROTOBUF_CONFIG_DIR "${TRITON_THIRD_PARTY_INSTALL_PREFIX}/protobuf/${LIB_DIR}/cmake/protobuf")
set(_FINDPACKAGE_UTF8_RANGE_CONFIG_DIR "${TRITON_THIRD_PARTY_INSTALL_PREFIX}/protobuf/${LIB_DIR}/cmake/utf8_range")

if (CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT)
set(TRITON_INSTALL_PREFIX ${CMAKE_CURRENT_BINARY_DIR}/install)
Expand Down Expand Up @@ -286,6 +287,7 @@ if(NOT TRITON_CORE_HEADERS_ONLY)
BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/triton-core"
CMAKE_CACHE_ARGS
-DProtobuf_DIR:PATH=${_FINDPACKAGE_PROTOBUF_CONFIG_DIR}
-Dutf8_range_DIR:PATH=${_FINDPACKAGE_UTF8_RANGE_CONFIG_DIR}
${_CMAKE_ARGS_OPENSSL_ROOT_DIR}
${_CMAKE_ARGS_CMAKE_TOOLCHAIN_FILE}
${_CMAKE_ARGS_VCPKG_TARGET_TRIPLET}
Expand Down
29 changes: 25 additions & 4 deletions src/model_config_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include "model_config_utils.h"

#include <google/protobuf/text_format.h>
#include <google/protobuf/util/json_util.h>
#include <google/protobuf/util/message_differencer.h>

Expand Down Expand Up @@ -754,7 +755,21 @@ GetNormalizedModelConfig(
RETURN_IF_ERROR(
AutoCompleteBackendFields(model_name, std::string(path), config));

LOG_PROTOBUF_VERBOSE(1, "Server side auto-completed config: ", (*config));
// Not using LOG_PROTOBUF_VERBOSE: it serializes via protobuf DebugString(),
// which protobuf v33 deliberately makes unstable (injects a
// "goo.gle/debugstr" marker) to discourage parsing. That marker is not valid
// text format and pollutes the logged config. Serialize with
// TextFormat::PrintToString for stable, parseable output.
if (LOG_VERBOSE_IS_ON(1)) {
std::string auto_completed_config;
google::protobuf::TextFormat::PrintToString(
*config, &auto_completed_config);
Comment on lines +765 to +766

@greptile-apps greptile-apps Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 TextFormat::PrintToString returns false when serialization fails (e.g., the message contains an unknown field that triggers strict printing). The return value is silently discarded here, so if it fails auto_completed_config will be empty and the log line will print nothing useful. Since this is only for diagnostics the cost is low, but a silent empty-string log can be confusing. Binding the result and logging a fallback is safer.

Suggested change
google::protobuf::TextFormat::PrintToString(
*config, &auto_completed_config);
if (!google::protobuf::TextFormat::PrintToString(
*config, &auto_completed_config)) {
auto_completed_config = "<serialization failed>";
}

triton::common::LogMessage(
__FILE__, __LINE__, triton::common::Logger::Level::kINFO,
"Server side auto-completed config: ", false)
.stream()
<< auto_completed_config;
Comment on lines +767 to +771

@greptile-apps greptile-apps Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The block is guarded by LOG_VERBOSE_IS_ON(1) but LogMessage is called with Logger::Level::kINFO. This means when verbose level 1 is enabled the auto-completed config is emitted as an INFO message, not a VERBOSE message, which could clutter INFO-level log streams. The rest of the file uses LOG_VERBOSE(1) << ... for the same pattern; using it here would match codebase conventions and emit at the correct level.

Suggested change
triton::common::LogMessage(
__FILE__, __LINE__, triton::common::Logger::Level::kINFO,
"Server side auto-completed config: ", false)
.stream()
<< auto_completed_config;
LOG_VERBOSE(1) << "Server side auto-completed config: "
<< auto_completed_config;

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

}

RETURN_IF_ERROR(NormalizeModelConfig(min_compute_capability, config));

Expand Down Expand Up @@ -1976,7 +1991,7 @@ CollectInt64Fields(
const google::protobuf::Reflection* refl = message->GetReflection();
for (int i = 0; i < desc->field_count(); ++i) {
const google::protobuf::FieldDescriptor* field = desc->field(i);
const std::string fullname = prefix + "::" + field->name();
const std::string fullname = prefix + "::" + std::string(field->name());
switch (field->type()) {
case google::protobuf::FieldDescriptor::TYPE_MESSAGE: {
if (field->is_repeated()) {
Expand Down Expand Up @@ -2201,9 +2216,15 @@ ModelConfigToJson(
std::string config_json_str;
::google::protobuf::util::JsonPrintOptions options;
options.preserve_proto_field_names = true;
options.always_print_primitive_fields = true;
::google::protobuf::util::MessageToJsonString(
options.always_print_fields_with_no_presence = true;
const auto to_json_status = ::google::protobuf::util::MessageToJsonString(
config, &config_json_str, options);
if (!to_json_status.ok()) {
return Status(
Status::Code::INTERNAL,
"failed to convert model configuration to JSON: " +
std::string(to_json_status.message()));
}

// We need to verify that every field 64-bit field in the
// ModelConfig protobuf is being handled. We hardcode the known
Expand Down
Loading