Skip to content

Commit

Permalink
Update logging to use Rust tracing_subscriber and
Browse files Browse the repository at this point in the history
avoid Python logger for Rust components.
  • Loading branch information
dnnanuti committed Mar 12, 2024
1 parent 69f32ff commit 792bacc
Show file tree
Hide file tree
Showing 6 changed files with 202 additions and 96 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
### Bug Fixes / Improvements
* Fix deadlock when enabling CRT debug logs. Removed former experimental method _enable_debug_logging().
* Refactor User-Agent setup for extensibility.
* Separate completely Rust logs and Python logs. Rust logs are configured through RUST_LOG,
S3_CONNECTOR_ENABLE_CRT_LOGS and S3_CONNECTOR_LOGS_DIR_PATH environment variables.

## v1.1.4 (February 26, 2024)

Expand Down
71 changes: 39 additions & 32 deletions s3torchconnectorclient/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions s3torchconnectorclient/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ built = "0.7"
pyo3 = { version = "0.19.2" }
pyo3-log = "0.8.3"
futures = "0.3.28"
mountpoint-s3-client = { version = "0.7.0", features = ["mock"] }
mountpoint-s3-crt = "0.6.1"
mountpoint-s3-client = { version = "0.8.0", features = ["mock"] }
mountpoint-s3-crt = "0.6.2"
log = "0.4.20"
tracing = { version = "0.1.40", default-features = false, features = ["std", "log"] }
tracing-subscriber = { version = "0.3.18", features = ["fmt", "env-filter"]}
Expand Down
105 changes: 90 additions & 15 deletions s3torchconnectorclient/python/tst/integration/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,16 @@
s3_uri = sys.argv[1]
region = sys.argv[2]
os.environ["ENABLE_CRT_LOGS"] = sys.argv[3]
logs_dir_path = sys.argv[4]
enable_crt_logs = sys.argv[3]
default_rust_log = sys.argv[4]
logs_dir_path = sys.argv[5]
if enable_crt_logs != "":
os.environ["S3_CONNECTOR_ENABLE_CRT_LOGS"] = enable_crt_logs
if default_rust_log != "":
os.environ["RUST_LOG"] = default_rust_log
if logs_dir_path != "":
os.environ["CRT_LOGS_DIR_PATH"] = logs_dir_path
os.environ["S3_CONNECTOR_LOGS_DIR_PATH"] = logs_dir_path
from s3torchconnector import S3MapDataset
Expand All @@ -35,7 +41,7 @@


@pytest.mark.parametrize(
"log_level, should_contain, should_not_contain",
"enable_crt_logs, should_contain, should_not_contain",
[
(
"info",
Expand Down Expand Up @@ -68,17 +74,19 @@
("OFF", ["INFO s3torchconnector.s3map_dataset"], ["awscrt"]),
],
)
def test_logging_valid(log_level, should_contain, should_not_contain, image_directory):
out, err = _start_subprocess(log_level, image_directory)
def test_crt_logging(
enable_crt_logs, should_contain, should_not_contain, image_directory
):
out, err = _start_subprocess(image_directory, enable_crt_logs=enable_crt_logs)
assert err == ""
assert all(s in out for s in should_contain)
assert all(s not in out for s in should_not_contain)


def test_logging_to_file(image_directory):
def test_logging_to_file_env_filters_unset(image_directory):
with tempfile.TemporaryDirectory() as log_dir:
print("Created temporary directory", log_dir)
out, err = _start_subprocess("INFO", image_directory, log_dir)
out, err = _start_subprocess(image_directory, logs_directory=log_dir)
# Standard output contains Python output
assert err == ""
assert "INFO s3torchconnector.s3map_dataset" in out
Expand All @@ -89,22 +97,89 @@ def test_logging_to_file(image_directory):
assert os.path.isfile(log_file)
with open(log_file) as f:
file_content = f.read()
assert all(s in file_content for s in ["awscrt", "INFO"])
assert all(
s not in file_content
for s in ["INFO s3torchconnector.s3map_dataset", "DEBUG", "TRACE"]
)
assert file_content == ""


def test_default_logging_env_filters_unset(image_directory):
out, err = _start_subprocess(image_directory)
# Standard output contains Python output
assert err == ""
assert "INFO s3torchconnector.s3map_dataset" in out
assert "awscrt" not in out


@pytest.mark.parametrize(
"enable_crt_logs, default_rust_log, out_should_contain, out_should_not_contain, file_should_contain, file_should_not_contain",
[
(
"INFO",
"",
["INFO s3torchconnector.s3map_dataset"],
["awscrt"],
["awscrt", "INFO"],
["INFO s3torchconnector.s3map_dataset", "DEBUG", "TRACE"],
),
(
"",
"DEBUG",
["INFO s3torchconnector.s3map_dataset"],
["awscrt"],
["DEBUG", "mountpoint_s3_client"],
["awscrt", "INFO s3torchconnector.s3map_dataset", "TRACE"],
),
(
"DEBUG",
"TRACE",
["INFO s3torchconnector.s3map_dataset"],
["awscrt"],
["DEBUG", "mountpoint_s3_client", "awscrt"],
["INFO s3torchconnector.s3map_dataset", "TRACE awscrt"],
),
],
)
def test_logging_to_file(
enable_crt_logs,
default_rust_log,
out_should_contain,
out_should_not_contain,
file_should_contain,
file_should_not_contain,
image_directory,
):
with tempfile.TemporaryDirectory() as log_dir:
print("Created temporary directory", log_dir)
out, err = _start_subprocess(
image_directory,
enable_crt_logs=enable_crt_logs,
default_rust_log=default_rust_log,
logs_directory=log_dir,
)
# Standard output contains Python output
assert err == ""
assert all(s in out for s in out_should_contain)
assert all(s not in out for s in out_should_not_contain)
files = os.listdir(log_dir)
assert len(files) == 1
log_file = os.path.join(log_dir, files[0])
assert os.path.isfile(log_file)
with open(log_file) as f:
file_content = f.read()
assert all(s in file_content for s in file_should_contain)
assert all(s not in file_content for s in file_should_not_contain)


def _start_subprocess(log_level, image_directory, logs_directory=""):
def _start_subprocess(
image_directory, *, enable_crt_logs="", default_rust_log="", logs_directory=""
):
process = subprocess.Popen(
[
sys.executable,
"-c",
PYTHON_TEST_CODE,
image_directory.s3_uri,
image_directory.region,
log_level,
enable_crt_logs,
default_rust_log,
logs_directory,
],
stdout=subprocess.PIPE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,27 @@
import hashlib
import logging
import math
import os
import pickle
import sys
import uuid
import random
import pytest

os.environ["S3_CONNECTOR_ENABLE_CRT_LOGS"] = "DEBUG"

from s3torchconnectorclient._mountpoint_s3_client import (
MountpointS3Client,
S3Exception,
ListObjectStream,
)

from s3torchconnectorclient import LOG_TRACE

logging.basicConfig(
format="%(levelname)s %(name)s %(asctime)-15s %(filename)s:%(lineno)d %(message)s"
)
logging.getLogger().setLevel(1)
logging.getLogger().setLevel(LOG_TRACE)

log = logging.getLogger(__name__)

Expand Down
Loading

0 comments on commit 792bacc

Please sign in to comment.