Skip to content
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.

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

Disable dynamic log file #7102

Merged
merged 1 commit into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
74 changes: 40 additions & 34 deletions qa/L0_logging/logging_endpoint_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import tritonclient.grpc as grpcclient
import tritonclient.http as httpclient
from google.protobuf import json_format
from tritonclient.utils import InferenceServerException


# Similar set up as dynamic batcher tests
Expand All @@ -49,7 +50,6 @@ def tearDown(self):
# tearDown() is properly executed and not affecting start state of
# other test cases
clear_settings = {
"log_file": "",
"log_info": True,
"log_warning": True,
"log_error": True,
Expand Down Expand Up @@ -126,84 +126,96 @@ def test_http_update_settings(self):
# by the server
self.check_server_initial_state()

expected_log_settings_1 = {
log_settings_1 = {
"log_file": "log_file.log",
"log_info": True,
"log_warning": True,
"log_error": True,
"log_verbose_level": 0,
"log_format": "default",
}
expected_log_settings_2 = {
"log_file": "log_file.log",
expected_log_settings_1 = {
"error": "updating log file was deprecated and no longer supported"
}

log_settings_2 = {
"log_info": False,
"log_warning": True,
"log_error": True,
"log_verbose_level": 0,
"log_format": "default",
}
expected_log_settings_3 = {
"log_file": "log_file.log",
expected_log_settings_2 = log_settings_2.copy()
expected_log_settings_2["log_file"] = ""

log_settings_3 = {
"log_info": False,
"log_warning": False,
"log_error": True,
"log_verbose_level": 0,
"log_format": "default",
}
expected_log_settings_4 = {
"log_file": "log_file.log",
expected_log_settings_3 = log_settings_3.copy()
expected_log_settings_3["log_file"] = ""

log_settings_4 = {
"log_info": False,
"log_warning": False,
"log_error": False,
"log_verbose_level": 0,
"log_format": "default",
}
expected_log_settings_5 = {
"log_file": "log_file.log",
expected_log_settings_4 = log_settings_4.copy()
expected_log_settings_4["log_file"] = ""

log_settings_5 = {
"log_info": False,
"log_warning": False,
"log_error": False,
"log_verbose_level": 1,
"log_format": "default",
}
expected_log_settings_6 = {
"log_file": "log_file.log",
expected_log_settings_5 = log_settings_5.copy()
expected_log_settings_5["log_file"] = ""

log_settings_6 = {
"log_info": False,
"log_warning": False,
"log_error": False,
"log_verbose_level": 1,
"log_format": "ISO8601",
}
expected_log_settings_6 = log_settings_6.copy()
expected_log_settings_6["log_file"] = ""

triton_client = httpclient.InferenceServerClient("localhost:8000")
self.assertEqual(
expected_log_settings_1,
triton_client.update_log_settings(settings=expected_log_settings_1),
"Unexpected updated log settings",
)
with self.assertRaisesRegex(
InferenceServerException, expected_log_settings_1["error"]
) as e:
triton_client.update_log_settings(settings=log_settings_1)
self.assertEqual(
expected_log_settings_2,
triton_client.update_log_settings(settings=expected_log_settings_2),
triton_client.update_log_settings(settings=log_settings_2),
"Unexpected updated log settings",
)
self.assertEqual(
expected_log_settings_3,
triton_client.update_log_settings(settings=expected_log_settings_3),
triton_client.update_log_settings(settings=log_settings_3),
"Unexpected updated log settings",
)
self.assertEqual(
expected_log_settings_4,
triton_client.update_log_settings(settings=expected_log_settings_4),
triton_client.update_log_settings(settings=log_settings_4),
"Unexpected updated log settings",
)
self.assertEqual(
expected_log_settings_5,
triton_client.update_log_settings(settings=expected_log_settings_5),
triton_client.update_log_settings(settings=log_settings_5),
"Unexpected updated log settings",
)
self.assertEqual(
expected_log_settings_6,
triton_client.update_log_settings(settings=expected_log_settings_6),
triton_client.update_log_settings(settings=log_settings_6),
"Unexpected updated log settings",
)

Expand All @@ -215,7 +227,6 @@ def test_grpc_update_settings(self):
triton_client = grpcclient.InferenceServerClient("localhost:8001")

log_settings_1 = {
"log_file": "log_file.log",
"log_info": True,
"log_warning": True,
"log_error": True,
Expand All @@ -227,7 +238,7 @@ def test_grpc_update_settings(self):
json.dumps(
{
"settings": {
"log_file": {"stringParam": "log_file.log"},
"log_file": {"stringParam": ""},
"log_info": {"boolParam": True},
"log_warning": {"boolParam": True},
"log_error": {"boolParam": True},
Expand All @@ -246,7 +257,6 @@ def test_grpc_update_settings(self):
)

log_settings_2 = {
"log_file": "log_file.log",
"log_info": False,
"log_warning": True,
"log_error": True,
Expand All @@ -258,7 +268,7 @@ def test_grpc_update_settings(self):
json.dumps(
{
"settings": {
"log_file": {"stringParam": "log_file.log"},
"log_file": {"stringParam": ""},
"log_info": {"boolParam": False},
"log_warning": {"boolParam": True},
"log_error": {"boolParam": True},
Expand All @@ -277,7 +287,6 @@ def test_grpc_update_settings(self):
)

log_settings_3 = {
"log_file": "log_file.log",
"log_info": False,
"log_warning": False,
"log_error": True,
Expand All @@ -289,7 +298,7 @@ def test_grpc_update_settings(self):
json.dumps(
{
"settings": {
"log_file": {"stringParam": "log_file.log"},
"log_file": {"stringParam": ""},
"log_info": {"boolParam": False},
"log_warning": {"boolParam": False},
"log_error": {"boolParam": True},
Expand All @@ -308,7 +317,6 @@ def test_grpc_update_settings(self):
)

log_settings_4 = {
"log_file": "log_file.log",
"log_info": False,
"log_warning": False,
"log_error": False,
Expand All @@ -320,7 +328,7 @@ def test_grpc_update_settings(self):
json.dumps(
{
"settings": {
"log_file": {"stringParam": "log_file.log"},
"log_file": {"stringParam": ""},
"log_info": {"boolParam": False},
"log_warning": {"boolParam": False},
"log_error": {"boolParam": False},
Expand All @@ -339,7 +347,6 @@ def test_grpc_update_settings(self):
)

log_settings_5 = {
"log_file": "log_file.log",
"log_info": False,
"log_warning": False,
"log_error": False,
Expand All @@ -351,7 +358,7 @@ def test_grpc_update_settings(self):
json.dumps(
{
"settings": {
"log_file": {"stringParam": "log_file.log"},
"log_file": {"stringParam": ""},
"log_info": {"boolParam": False},
"log_warning": {"boolParam": False},
"log_error": {"boolParam": False},
Expand All @@ -370,7 +377,6 @@ def test_grpc_update_settings(self):
)

log_settings_6 = {
"log_file": "log_file.log",
"log_info": False,
"log_warning": False,
"log_error": False,
Expand All @@ -382,7 +388,7 @@ def test_grpc_update_settings(self):
json.dumps(
{
"settings": {
"log_file": {"stringParam": "log_file.log"},
"log_file": {"stringParam": ""},
"log_info": {"boolParam": False},
"log_warning": {"boolParam": False},
"log_error": {"boolParam": False},
Expand Down
9 changes: 7 additions & 2 deletions qa/L0_logging/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,12 @@ rm -f ./curl.out
code=`curl -s -w %{http_code} -o ./curl.out -d'{"log_file":"other_log.log"}' localhost:8000/v2/logging`
set +e

verify_correct_settings "other_log.log" "true" "true" "true" "1" "default"
# updating log file location no longer supported
if [ `grep -c "\"error\":\"updating log file was deprecated and no longer supported\"" ./curl.out` != "1" ]; then
echo -e "\n***\n*** Test Failed: Incorrect Error Response\n***"
RET=1
fi
verify_correct_settings "log_file.log" "true" "true" "true" "1" "default"

$SIMPLE_HTTP_CLIENT >> client_test_log_file.log 2>&1
if [ $? -ne 0 ]; then
Expand All @@ -225,7 +230,7 @@ if [ $actual_log_count -lt $expected_log_count ]; then
RET=1
fi
expected_other_log_count=31
actual_other_log_count=$(grep -c ^[IWEV][0-9][0-9][0-9][0-9].* ./other_log.log)
actual_other_log_count=$(grep -c ^[IWEV][0-9][0-9][0-9][0-9].* ./log_file.log)
if [ $actual_other_log_count -lt $expected_other_log_count ]; then
echo $actual_other_log_count
echo $expected_other_log_count
Expand Down
30 changes: 4 additions & 26 deletions src/grpc/grpc_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1463,32 +1463,10 @@ CommonHandler::RegisterLogging()
static std::string setting_name = "log_file";
auto it = request.settings().find(setting_name);
if (it != request.settings().end()) {
const auto& log_param = it->second;
if (log_param.parameter_choice_case() !=
inference::LogSettingsRequest_SettingValue::ParameterChoiceCase::
kStringParam) {
err = TRITONSERVER_ErrorNew(
TRITONSERVER_ERROR_INVALID_ARG,
(std::string("expect string for '") + setting_name + "'")
.c_str());
GOTO_IF_ERR(err, earlyexit);
} else {
// Set new settings in server then in core
const std::string& log_file_path = it->second.string_param();
const std::string& error = LOG_SET_OUT_FILE(log_file_path);
if (!error.empty()) {
err = TRITONSERVER_ErrorNew(
TRITONSERVER_ERROR_INTERNAL, (error).c_str());
GOTO_IF_ERR(err, earlyexit);
}
// Okay to pass nullptr because we know the update will be applied
// to the global object.
err = TRITONSERVER_ServerOptionsSetLogFile(
nullptr, log_file_path.c_str());
if (err != nullptr) {
GOTO_IF_ERR(err, earlyexit);
}
}
err = TRITONSERVER_ErrorNew(
TRITONSERVER_ERROR_UNSUPPORTED,
"updating log file was deprecated and no longer supported");
GOTO_IF_ERR(err, earlyexit);
}
}
{
Expand Down
25 changes: 5 additions & 20 deletions src/http_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2080,7 +2080,6 @@ HTTPAPIServer::HandleLogging(evhtp_request_t* req)
"unexpected error getting dynamic logging request buffers"));
}
}
TRITONSERVER_Error* err = nullptr;
triton::common::TritonJson::Value request;
size_t buffer_len = evbuffer_get_length(req->buffer_in);
RETURN_AND_RESPOND_IF_ERR(
Expand All @@ -2090,25 +2089,11 @@ HTTPAPIServer::HandleLogging(evhtp_request_t* req)
triton::common::TritonJson::Value setting_json;
if (request.Find("log_file", &setting_json)) {
if (!setting_json.IsNull()) {
// Set new settings in server then in core
std::string log_file_path;
RETURN_AND_RESPOND_IF_ERR(req, setting_json.AsString(&log_file_path));
const std::string& error = LOG_SET_OUT_FILE(log_file_path);
if (!error.empty()) {
RETURN_AND_RESPOND_IF_ERR(
req, TRITONSERVER_ErrorNew(
TRITONSERVER_ERROR_UNAVAILABLE, (error).c_str()));
}
// Okay to pass nullptr because we know the update will be applied
// to the global object.
err = TRITONSERVER_ServerOptionsSetLogFile(
nullptr, log_file_path.c_str());
if (err != nullptr) {
RETURN_AND_RESPOND_IF_ERR(
req, TRITONSERVER_ErrorNew(
TRITONSERVER_ERROR_UNAVAILABLE,
(TRITONSERVER_ErrorMessage(err))));
}
RETURN_AND_RESPOND_IF_ERR(
req,
TRITONSERVER_ErrorNew(
TRITONSERVER_ERROR_UNSUPPORTED,
"updating log file was deprecated and no longer supported"));
}
}
if (request.Find("log_info", &setting_json)) {
Expand Down
Loading