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

Logging for SmartRedis #281

Merged
merged 41 commits into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
7a0e172
Initial C++ implementation
billschereriii Nov 16, 2022
798d305
Updates from Chris's comments
billschereriii Nov 17, 2022
15b17ae
4-client implementation; doc updates
billschereriii Nov 17, 2022
d9f0657
Merge
billschereriii Nov 18, 2022
6f6a2aa
Address Matt's feedback and eliminate chicken-and-egg logging issues
billschereriii Nov 18, 2022
86b3043
Fix compile errors
billschereriii Nov 22, 2022
fb3dd7d
Fix link issue that seems to only affect MacOS
billschereriii Nov 22, 2022
c19ae42
It compiles so it must be fine
billschereriii Nov 23, 2022
fd0a455
Python logging is code complete
billschereriii Nov 23, 2022
4e257c8
All tests pass again
billschereriii Nov 25, 2022
6aa84bb
Lager fortran fixes (#1)
ashao Nov 28, 2022
3ee80ee
Add missing newlines to logging
billschereriii Nov 28, 2022
1cb125f
Merge Andrew's Fortran fixes
billschereriii Nov 28, 2022
b9211f1
Update test cases to configure logging ID
billschereriii Nov 28, 2022
38f41d3
Update examples to register client for logging
billschereriii Nov 28, 2022
7b984ca
Tweak CI/CD YAML files to set up logging environment variables
billschereriii Nov 28, 2022
e31a122
__FILE__ is too unpredictable to use in Fortran
billschereriii Nov 28, 2022
46457e3
Move client create/destroy to debug level to reduce contention on sta…
billschereriii Nov 28, 2022
02ca745
Fix context fixture; catch a few more anonymous tests
billschereriii Nov 29, 2022
a97d7da
Use INFO-level logging for CI/CD; __FILE__ is too long for C++ client ID
billschereriii Nov 29, 2022
92a4590
Make log methods accessible in Python
billschereriii Nov 29, 2022
e49b3fb
Python logger tests
billschereriii Nov 29, 2022
fb6dc82
Add CPP unit tests; fix environment corruption in existing unit tests…
billschereriii Nov 29, 2022
4a321cd
Fortran unit test
billschereriii Nov 29, 2022
50881ba
C unit test
billschereriii Nov 29, 2022
950030e
Merge
billschereriii Nov 30, 2022
80fe22f
merge
billschereriii Dec 5, 2022
012112f
Easy parts of the review feedback
billschereriii Dec 6, 2022
654ab90
Clean up test case
billschereriii Dec 6, 2022
b8432ee
New base class SRObject for Client, DataSet, LogContext
billschereriii Dec 6, 2022
ad81e1f
add virtual destructors
billschereriii Dec 7, 2022
0c2e096
Updates from MattD feedback
billschereriii Dec 7, 2022
c6530ca
Fix error?
billschereriii Dec 7, 2022
973a8e2
C++ tests fully working. Some python tests segfaulting
billschereriii Dec 8, 2022
0a4f5ca
Python tests working again
billschereriii Dec 9, 2022
c836a1d
Remove unneeded logging
billschereriii Dec 9, 2022
894bd40
C tests work again
billschereriii Dec 9, 2022
dbd1d11
Fortran interface for LogContext
billschereriii Dec 9, 2022
ca12f0e
Fortran logging works now
billschereriii Dec 9, 2022
3ffbe06
Eliminate hard-coded string lengths
billschereriii Dec 14, 2022
79b78ec
Cleaner way to handle default logger_name
billschereriii Dec 14, 2022
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 .github/workflows/run_post_merge_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -140,5 +140,7 @@ jobs:
if: contains(matrix.os, 'ubuntu') && !contains(matrix.compiler, 'intel')
run: |
export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$PWD/install/lib &&
export SR_LOG_FILE=smartredis_examples_log.txt &&
export SR_LOG_LEVEL=INFO &&
make test-examples

4 changes: 4 additions & 0 deletions .github/workflows/run_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ jobs:
bash ../build-scripts/build-catch.sh &&
cd ../ &&
export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$PWD/install/lib &&
export SR_LOG_FILE=smartredis_cicd_tests_log.txt &&
export SR_LOG_LEVEL=INFO &&
make test-verbose

- name: Run Python coverage tests
Expand Down Expand Up @@ -163,5 +165,7 @@ jobs:
- name: Run testing with redis cluster
run: |
export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$PWD/install/lib &&
export SR_LOG_FILE=smartredis_cicd_tests_log.txt &&
export SR_LOG_LEVEL=INFO &&
make test-verbose

2 changes: 2 additions & 0 deletions .github/workflows/run_tests_uds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -145,5 +145,7 @@ jobs:
- name: Run testing with UDS redis
run: |
export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$PWD/install/lib &&
export SR_LOG_FILE=smartredis_uds_tests_log.txt &&
export SR_LOG_LEVEL=INFO &&
make test-verbose

12 changes: 10 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ set(CLIENT_SRC
src/c/c_client.cpp
src/c/c_dataset.cpp
src/c/c_error.cpp
src/c/c_logcontext.cpp
src/cpp/address.cpp
src/cpp/client.cpp
src/cpp/dataset.cpp
Expand Down Expand Up @@ -87,6 +88,9 @@ set(CLIENT_SRC
src/cpp/stringfield.cpp
src/cpp/pipelinereply.cpp
src/cpp/threadpool.cpp
src/cpp/utility.cpp
src/cpp/logger.cpp
src/cpp/srobject.cpp
)

include_directories(SYSTEM
Expand All @@ -96,10 +100,12 @@ include_directories(SYSTEM

if (BUILD_FORTRAN)
set(FORTRAN_SRC
src/fortran/fortran_c_interop.F90
src/fortran/dataset.F90
src/fortran/client.F90
src/fortran/dataset.F90
src/fortran/errors.F90
src/fortran/fortran_c_interop.F90
src/fortran/logcontext.F90
src/fortran/logger.F90
)
include_directories(src/fortran)
# Note the following has to be before ANY add_library command)
Expand Down Expand Up @@ -137,6 +143,8 @@ if(BUILD_PYTHON)
add_library(smartredis_static STATIC ${CLIENT_SRC})

pybind11_add_module(smartredisPy
src/python/src/pysrobject.cpp
src/python/src/pylogcontext.cpp
src/python/src/pyclient.cpp
src/python/src/pydataset.cpp
${CLIENT_SRC}
Expand Down
4 changes: 4 additions & 0 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ def mock_data():
def mock_model():
return MockTestModel

@pytest.fixture
def context(request):
return request.node.name

class MockTestData:

@staticmethod
Expand Down
3 changes: 3 additions & 0 deletions doc/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Description

- Improved error reporting capabilities for Fortran clients
- Python error messages from SmartRedis contain more information
- Added logging functionality to the SmartRedis library
- A bug related to thread pool initialization was fixed.
- This version adds new functionality in the form of support for Unix Domain Sockets.
- Fortran client can now be optionally built with the rest of the library
Expand All @@ -23,6 +24,7 @@ Detailed Notes

- Fortran clients can now access error text and source location (PR284_)
- Add exception location information from CPP code to Python exceptions (PR283_)
- Added client activity and manual logging for developer use (PR281_)
- Fix thread pool error (PR280_)
- Update library linking instructions and update Fortran tester build process (PR277_)
- Added `add_metadata_for_xarray` and `transform_to_xarray` methods in `DatasetConverter` class for initial support with Xarray (PR262_)
Expand All @@ -32,6 +34,7 @@ Detailed Notes

.. _PR280: https://github.com/CrayLabs/SmartRedis/pull/284
.. _PR280: https://github.com/CrayLabs/SmartRedis/pull/283
.. _PR281: https://github.com/CrayLabs/SmartRedis/pull/281
.. _PR280: https://github.com/CrayLabs/SmartRedis/pull/280
.. _PR277: https://github.com/CrayLabs/SmartRedis/pull/277
.. _PR262: https://github.com/CrayLabs/SmartRedis/pull/262
Expand Down
30 changes: 30 additions & 0 deletions doc/runtime.rst
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,36 @@ location. However, the Python ``Client`` constructor also allows
for the database location to be set as an input parameter. In
this case, it sets SSDB from the input parameter.

Logging Environment Variables
=============================

SmartRedis will log events to a file on behalf of a client. There
are two main environment variables that affect logging, ``SR_LOG_FILE``
and ``SR_LOG_LEVEL``.

``SR_LOG_FILE`` is the specifier for the location of the file that
receives logging information. Each entry in the file will be prefixed
with a timestamp and the identifier of the client that invoked the logging
message. The path may be relative or absolute, though a relative path risks
creation of multiple log files if the executables that instantiate SmartRedis
clients are run from different directories. If this variable is not set,
logging information will be sent to standard console output (STDOUT in C and
C++; output_unit in Fortran).

``SR_LOG_LEVEL`` relates to the verbosity of information that wil be logged.
It may be one of three levels: ``QUIET`` disables logging altogether.
``INFO`` provides informational logging, such as exception events that
transpire within the SmartRedis library and creation or destruction of a
client object. ``DEBUG`` provides more verbose logging, including information
on the activities of the SmartRedis thread pool and API function entry and exit.
Debug level logging will also log the absence of an expected environment variable,
though this can happen only if the variables to set up logging are in place. If
this parameter is not set, a default logging level of ``INFO`` will be adopted.

The runtime impact of log levels NONE or INFO should be minimal on
client performance; however, seting the log level to DEBUG may cause some
degradation.

Ensemble Environment Variables
==============================

Expand Down
4 changes: 3 additions & 1 deletion examples/parallel/cpp/smartredis_mnist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,12 @@ int main(int argc, char* argv[]) {
// Retrieve the MPI rank
int rank;
MPI_Comm_rank(MPI_COMM_WORLD, &rank);
std::string logger_name("Client ");
logger_name += std::to_string(rank);

// Initialize a Client object
bool cluster_mode = true; // Set to false if not using a clustered database
SmartRedis::Client client(cluster_mode);
SmartRedis::Client client(cluster_mode, logger_name);

// Set the model and script that will be used by all ranks
// from MPI rank 0.
Expand Down
10 changes: 7 additions & 3 deletions examples/parallel/cpp/smartredis_put_get_3D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,17 @@ int main(int argc, char* argv[]) {
for(size_t i=0; i<n_values; i++)
input_tensor[i] = 2.0*rand()/RAND_MAX - 1.0;

// Get our rank
int rank = 0;
MPI_Comm_rank(MPI_COMM_WORLD, &rank);
std::string logger_name("Client ");
logger_name += std::to_string(rank);

// Initialize a SmartRedis client
bool cluster_mode = true; // Set to false if not using a clustered database
SmartRedis::Client client(cluster_mode);
SmartRedis::Client client(cluster_mode, logger_name);

// Put the tensor in the database
int rank = 0;
MPI_Comm_rank(MPI_COMM_WORLD, &rank);
std::string key = "3d_tensor_" + std::to_string(rank);
client.put_tensor(key, input_tensor.data(), dims,
SRTensorTypeDouble, SRMemLayoutContiguous);
Expand Down
2 changes: 1 addition & 1 deletion examples/parallel/fortran/smartredis_dataset.F90
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ program main
if (result .ne. SRNoError) error stop 'dataset%get_meta_scalars failed'

! Initialize a client
result = client%initialize(.true.) ! Change .false. to .true. if not using a clustered database
result = client%initialize(.true., "smartredis_dataset") ! Change .false. to .true. if not using a clustered database
if (result .ne. SRNoError) error stop 'client%initialize failed'

! Send the dataset to the database via the client
Expand Down
2 changes: 1 addition & 1 deletion examples/parallel/fortran/smartredis_mnist.F90
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ program mnist_example
write(key_suffix, "(A,I1.1)") "_",pe_id

! Initialize a client
result = client%initialize(.true.) ! Change .false. to .true. if not using a clustered database
result = client%initialize(.true., "smartredis_mnist") ! Change .false. to .true. if not using a clustered database
if (result .ne. SRNoError) error stop 'client%initialize failed'

! Set up model and script for the computation
Expand Down
2 changes: 1 addition & 1 deletion examples/parallel/fortran/smartredis_put_get_3D.F90
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ program main
call random_number(recv_array_real_64)

! Initialize a client
result = client%initialize(.true.) ! Change .false. to .true. if not using a clustered database
result = client%initialize(.true., "smartredis_put_get_3D") ! Change .false. to .true. if not using a clustered database
if (result .ne. SRNoError) error stop 'client%initialize failed'

! Add a tensor to the database and verify that we can retrieve it
Expand Down
4 changes: 3 additions & 1 deletion examples/serial/c/example_put_get_3D.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ int main(int argc, char* argv[]) {
values.
*/

const char* logger_name = "put_get_3d";
size_t cid_len = strlen(logger_name);
size_t n_dims = 3;
size_t* dims = malloc(n_dims*sizeof(size_t));
dims[0] = 10;
Expand All @@ -46,7 +48,7 @@ int main(int argc, char* argv[]) {

void* client = NULL;
bool cluster_mode = true; // Set to false if not using a clustered database
if (SRNoError != SmartRedisCClient(cluster_mode, &client)) {
if (SRNoError != SmartRedisCClient(cluster_mode, logger_name, cid_len, &client)) {
printf("Client initialization failed!\n");
exit(-1);
}
Expand Down
4 changes: 3 additions & 1 deletion examples/serial/c/example_put_unpack_1D.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@

int main(int argc, char* argv[]) {

const char* logger_name = "put_unpack_1d";
size_t cid_len = strlen(logger_name);
size_t* dims = malloc(sizeof(size_t));
dims[0] = 10;
size_t n_dims = 1;
Expand All @@ -46,7 +48,7 @@ int main(int argc, char* argv[]) {

void* client = NULL;
bool cluster_mode = true; // Set to false if not using a clustered database
if (SRNoError != SmartRedisCClient(cluster_mode, &client)) {
if (SRNoError != SmartRedisCClient(cluster_mode, logger_name, cid_len, &client)) {
printf("Client initialization failed!\n");
exit(-1);
}
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/cpp/smartredis_dataset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ int main(int argc, char* argv[]) {

// Initialize a SmartRedis client
bool cluster_mode = true; // Set to false if not using a clustered database
SmartRedis::Client client(cluster_mode);
SmartRedis::Client client(cluster_mode, __FILE__);

// Create a DataSet
SmartRedis::DataSet dataset("example_dataset");
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/cpp/smartredis_mnist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ int main(int argc, char* argv[]) {
// In general, Client objects should be reused, but for this
// small example, Client objects are not reused.
bool cluster_mode = true; // Set to false if not using a clustered database
SmartRedis::Client client(cluster_mode);
SmartRedis::Client client(cluster_mode, __FILE__);

// Build model key, file name, and then set model
// from file using client API
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/cpp/smartredis_model.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ int main(int argc, char* argv[]) {

// Initialize a SmartRedis client to connect to the Redis database
bool cluster_mode = true; // Set to false if not using a clustered database
SmartRedis::Client client(cluster_mode);
SmartRedis::Client client(cluster_mode, __FILE__);

// Use the client to set a model in the database from a file
std::string model_key = "mnist_model";
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/cpp/smartredis_put_get_3D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ int main(int argc, char* argv[]) {

// Initialize a SmartRedis client
bool cluster_mode = true; // Set to false if not using a clustered database
SmartRedis::Client client(cluster_mode);
SmartRedis::Client client(cluster_mode, __FILE__);

// Put the tensor in the database
std::string key = "3d_tensor";
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/fortran/smartredis_dataset.F90
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ program main
if (result .ne. SRNoError) error stop 'dataset%get_meta_scalars failed'

! Initialize a client
result = client%initialize(.true.) ! Change .false. to .true. if not using a clustered database
result = client%initialize(.true., "smartredis_dataset") ! Change .false. to .true. if not using a clustered database
if (result .ne. SRNoError) error stop 'client%initialize failed'

! Send the dataset to the database via the client
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/fortran/smartredis_put_get_3D.F90
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ program main
call random_number(send_array_real_64)

! Initialize a client
result = client%initialize(.true.) ! Change .false. to .true. if not using a clustered database
result = client%initialize(.true., "smartredis_put_get_3D") ! Change .false. to .true. if not using a clustered database
if (result .ne. SRNoError) error stop 'client%initialize failed'

! Send a tensor to the database via the client and verify that we can retrieve it
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/python/example_model_file_torch.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def forward(self, x):

# Connect a SmartRedis client
db_address = "127.0.0.1:6379"
client = Client(address=db_address, cluster=True)
client = Client(address=db_address, cluster=True, logger_name="example_model_file_torch.py")

try:
net = Net()
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/python/example_model_torch.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def forward(self, x):

# Connect a SmartRedis client and set the model in the database
db_address = "127.0.0.1:6379"
client = Client(address=db_address, cluster=True)
client = Client(address=db_address, cluster=True, logger_name="example_model_torch.py")
client.set_model("torch_cnn", model, "TORCH", "CPU")

# Retrieve the model and verify that the retrieved
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/python/example_put_get_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@

# Connect SmartRedis client to Redis database
db_address = "127.0.0.1:6379"
client = Client(address=db_address, cluster=True)
client = Client(address=db_address, cluster=True, logger_name="example_put_get_dataset.py")

# Place the DataSet into the database
client.put_dataset(dataset)
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/python/example_put_get_tensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

# Connect a SmartRedis client to Redis database
db_address = "127.0.0.1:6379"
client = Client(address=db_address, cluster=True)
client = Client(address=db_address, cluster=True, logger_name="example_put_get_tensor.py")

# Send a 2D tensor to the database
key = "2D_array"
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/python/example_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def two_to_one(data, data_2):

# Connect a SmartRedis client to the Redis database
db_address = "127.0.0.1:6379"
client = Client(address=db_address, cluster=True)
client = Client(address=db_address, cluster=True, logger_name="example_script.py")

# Generate some test data to feed to the two_to_one function
data = np.array([[1, 2, 3, 4]])
Expand Down
2 changes: 1 addition & 1 deletion examples/serial/python/example_script_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

# Connect to the Redis database
db_address = "127.0.0.1:6379"
client = Client(address=db_address, cluster=True)
client = Client(address=db_address, cluster=True, logger_name="example_script_file.py")

# Place the script in the database
client.set_script_from_file(
Expand Down
8 changes: 7 additions & 1 deletion include/c_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,16 @@ extern "C" {
/*!
* \brief C-client constructor
* \param cluster Flag to indicate if a database cluster is being used
* \param logger_name Identifier for the current client
* \param logger_name_length Length in characters of the logger_name string
* \param new_client Receives the new client
* \return Returns SRNoError on success or an error code on failure
*/
SRError SmartRedisCClient(bool cluster, void **new_client);
SRError SmartRedisCClient(
bool cluster,
const char* logger_name,
const size_t logger_name_length,
void **new_client);

/*!
* \brief C-client destructor
Expand Down
Loading