Skip to content

Commit

Permalink
test_utils: reset logger ostream pointer in tee logger
Browse files Browse the repository at this point in the history
The tee_wrapper helper was being used to set the ostream pointer for
global logger to be an ostream instance that lived only on the stack,
and not resetting it to something global like std::cerr after the test
finished. This resulted in the following segfault in other tests that
ran after the change but didn't make their own change to ostream
pointer:

```
  #0 0x7f875b0bbc1f in std::__1::basic_ostream<char, std::__1::char_traits<char> >::sentry::sentry(std::__1::basic_ostream<char, std::__1::char_traits<char> >&) (/vectorized/lib/libc++.so.1+0x65c1f)
  #1 0x5597c3979afc in std::__1::basic_ostream<char, std::__1::char_traits<char> >& std::__1::__put_character_sequence<char, std::__1::char_traits<char> >(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, char const*, unsigned long) /vectorized/llvm/bin/../include/c++/v1/ostream:722:57
  #2 0x5597c4079e44 in std::__1::basic_ostream<char, std::__1::char_traits<char> >& std::__1::operator<<<char, std::__1::char_traits<char> >(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, std::__1::basic_string_view<char, std::__1::char_traits<char> >) /vectorized/llvm/bin/../include/c++/v1/ostream:1057:12
  #3 0x5597c602c9f3 in seastar::logger::do_log(seastar::log_level, seastar::logger::log_writer&) /v/build/v_deps_build/seastar-prefix/src/seastar/src/util/log.cc:342:15
  #4 0x5597c3c4ad20 in void seastar::logger::log<char const*, int, seastar::basic_sstring<char, unsigned int, 15u, true>&>(seastar::log_level, seastar::logger::format_info, char const*&&, int&&, seastar::basic_sstring<char, unsigned int, 15u, true>&) /vectorized/include/seastar/util/log.hh:231:17
  #5 0x5597c3f23c51 in void seastar::logger::trace<char const*, int, seastar::basic_sstring<char, unsigned int, 15u, true>&>(seastar::logger::format_info, char const*&&, int&&, seastar::basic_sstring<char, unsigned int, 15u, true>&) /vectorized/include/seastar/util/log.hh:385:9
  #6 0x5597c3f14b95 in cloud_roles::signature_v4::sign_header(boost::beast::http::header<true, boost::beast::http::basic_fields<std::__1::allocator<char> > >&, std::__1::basic_string_view<char, std::__1::char_traits<char> >) const /var/lib/buildkite-agent/builds/buildkite-amd64-builders-i-00e717c5e253e2891-1/redpanda/vtools/src/v/cloud_roles/signature.cc:382:5
  #7 0x5597c3bf1dfb in cloud_roles::apply_aws_credentials::add_auth(boost::beast::http::header<true, boost::beast::http::basic_fields<std::__1::allocator<char> > >&) const /var/lib/buildkite-agent/builds/buildkite-amd64-builders-i-00e717c5e253e2891-1/redpanda/vtools/src/v/cloud_roles/apply_aws_credentials.cc:83:23
  #8 0x5597c3b8f88d in cloud_roles::apply_credentials::add_auth(boost::beast::http::header<true, boost::beast::http::basic_fields<std::__1::allocator<char> > >&) const /var/lib/buildkite-agent/builds/buildkite-amd64-builders-i-00e717c5e253e2891-1/redpanda/vtools/src/v/cloud_roles/apply_credentials.h:47:23
  #9 0x5597c3b8c578 in test_aws_headers::test_method() /var/lib/buildkite-agent/builds/buildkite-amd64-builders-i-00e717c5e253e2891-1/redpanda/vtools/src/v/cloud_roles/tests/credential_tests.cc:50:17
```

This change resets the ostream pointer in the wrapper destructor.

Signed-off-by: Noah Watkins <[email protected]>
  • Loading branch information
dotnwat committed Apr 10, 2023
1 parent 864ff90 commit cc1e787
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 11 deletions.
12 changes: 4 additions & 8 deletions src/v/cloud_roles/tests/fetch_credentials_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,7 @@ FIXTURE_TEST(test_short_lived_sts_credentials, http_imposter_fixture) {
}

FIXTURE_TEST(test_client_closed_on_error, http_imposter_fixture) {
tee_wrapper wrapper;
cloud_roles::clrl_log.set_ostream(wrapper.stream);
tee_wrapper wrapper(cloud_roles::clrl_log);

fail_request_if(
[](const auto&) { return true; },
Expand Down Expand Up @@ -379,8 +378,7 @@ FIXTURE_TEST(test_handle_temporary_timeout, http_imposter_fixture) {
// refresh operation will attempt to retry. In order not to expose the retry
// counter or make similar changes to the class just for testing, this test
// scans the log for the message emitted when a ss::timed_out_error is seen.
tee_wrapper wrapper;
cloud_roles::clrl_log.set_ostream(wrapper.stream);
tee_wrapper wrapper(cloud_roles::clrl_log);
ss::abort_source as;
std::optional<cloud_roles::credentials> c;

Expand Down Expand Up @@ -408,8 +406,7 @@ FIXTURE_TEST(test_handle_temporary_timeout, http_imposter_fixture) {
}

FIXTURE_TEST(test_handle_bad_response, http_imposter_fixture) {
tee_wrapper wrapper;
cloud_roles::clrl_log.set_ostream(wrapper.stream);
tee_wrapper wrapper(cloud_roles::clrl_log);

fail_request_if(
[](const auto&) { return true; },
Expand Down Expand Up @@ -449,8 +446,7 @@ FIXTURE_TEST(test_intermittent_error, http_imposter_fixture) {
// successful request. The refresh credentials object should retry after the
// first failure.

tee_wrapper wrapper;
cloud_roles::clrl_log.set_ostream(wrapper.stream);
tee_wrapper wrapper(cloud_roles::clrl_log);

when()
.request(cloud_role_tests::aws_role_query_url)
Expand Down
21 changes: 18 additions & 3 deletions src/v/test_utils/tee_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,43 @@

#pragma once

#include <seastar/util/log.hh>

#include <boost/iostreams/stream.hpp>
#include <boost/iostreams/tee.hpp>

#include <iostream>
#include <sstream>

/*
* A utility for capturing seastar::logger output.
*
* TODO: ideally this introspects the installed logger so that the proper logger
* could be reinstalled in the destructor. The default logger for seastar is
* std::cerr which is used here. As long as this utility is only used in unit
* tests then this default behavior should be sufficient.
*/
struct tee_wrapper {
using device_t
= boost::iostreams::tee_device<std::ostream, std::stringstream>;

using stream_t = boost::iostreams::stream<device_t>;

tee_wrapper()
explicit tee_wrapper(seastar::logger& logger)
: sstr{}
, device{std::cout, sstr}
, stream{device} {}
, device{std::cerr, sstr}
, stream{device}
, logger(logger) {
logger.set_ostream(stream);
}

tee_wrapper(const tee_wrapper&) = delete;
tee_wrapper& operator=(const tee_wrapper&) = delete;
tee_wrapper(tee_wrapper&&) = delete;
tee_wrapper& operator=(tee_wrapper&&) = delete;

~tee_wrapper() {
logger.set_ostream(std::cerr);
if (stream.is_open()) {
stream->flush();
stream->close();
Expand All @@ -45,4 +59,5 @@ struct tee_wrapper {
std::stringstream sstr;
device_t device;
stream_t stream;
seastar::logger& logger;
};

0 comments on commit cc1e787

Please sign in to comment.