Skip to content
This repository has been archived by the owner on Jul 24, 2021. It is now read-only.

Commit

Permalink
move "audit" feature into the logging config section as the "verbose"…
Browse files Browse the repository at this point in the history
… option
  • Loading branch information
karenetheridge committed Jan 27, 2020
1 parent 4129a40 commit de9856d
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 29 deletions.
5 changes: 3 additions & 2 deletions conch.conf.dist
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
},

features => {
audit => 0, # turns on logging of request and response bodies
rollbar => 0,
nytprof => 0,

Expand Down Expand Up @@ -140,12 +139,14 @@

# level => 'debug', # default
# bunyan => 1, # default
# with_trace => 0, # default
# with_trace => 0, # default; enabled when verbose => 1

# For docker, this should probably be set to 1. If not, logs will be
# written to /app/conch/log inside the container and a volume will need
# to be mounted there.
log_to_stderr => 0,

verbose => 0, # turns on logging of request and response bodies, and exception stack traces
},

}
8 changes: 4 additions & 4 deletions lib/Conch/Plugin/Logging.pm
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ sub register ($self, $app, $config) {
die 'unrecognized log level '.$plugin_config->{level}
if $plugin_config->{level} and not exists $LEVEL{$plugin_config->{level}};

my $log_to_stderr = delete $plugin_config->{log_to_stderr};
my ($log_to_stderr, $verbose) = delete $plugin_config->@{qw(log_to_stderr verbose)};

my %log_args = (
level => 'debug',
bunyan => 1,
$app->feature('audit') ? ( with_trace => 1 ) : (),
$verbose ? ( with_trace => 1 ) : (),
$plugin_config->%*,
);

Expand Down Expand Up @@ -104,14 +104,14 @@ sub register ($self, $app, $config) {
headers => $req_headers,
query_params => $c->req->query_params->to_hash,
# no body_params: presently we do not permit application/x-www-form-urlencoded
$c->feature('audit') && !(ref $req_json eq 'HASH' and exists $req_json->{password})
$verbose && !(ref $req_json eq 'HASH' and exists $req_json->{password})
? ( body => $c->req->json // $c->req->text ) : (),
},
res => {
headers => $res_headers,
statusCode => $c->res->code,
$c->res->code >= 400
|| ($c->feature('audit') && !(ref $res_json eq 'HASH' and grep /token/, keys $res_json->%*))
|| ($verbose && !(ref $res_json eq 'HASH' and grep /token/, keys $res_json->%*))
? ( body => $c->res->json // $c->res->text ) : (),
},
latency => int(1000 * $c->timing->elapsed('request_latency')),
Expand Down
8 changes: 6 additions & 2 deletions lib/Test/Conch.pm
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ sub new {
my $self = Test::Mojo->new(
Conch => {
features => {
audit => 1,
no_db => ($pg ? 0 : 1),
($args->{config}//{})->{features} ? delete($args->{config}{features})->%* : (),
},
Expand All @@ -115,8 +114,13 @@ sub new {
from_host => 'joyent.com',
($args->{config}//{})->{mail} ? delete($args->{config}{mail})->%* : (),
},
logging => {
max_history_size => 50,
verbose => 1,
($args->{config}//{})->{logging} ? delete($args->{config}{logging})->%* : (),
},

secrets => ['********'],
logging => { max_history_size => 50 },

$args->{config} ? delete($args->{config})->%* : (),
}
Expand Down
30 changes: 11 additions & 19 deletions t/conch-log.t
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ sub add_test_routes ($t) {
reset_log;

cmp_deeply(
Test::Conch->new(config => { features => { audit => 0, no_db => 1 } })->app->log,
Test::Conch->new(config => { features => { no_db => 1 }, logging => { verbose => 0 } })->app->log,
all(
isa('Conch::Log'),
methods(
Expand All @@ -176,7 +176,7 @@ sub add_test_routes ($t) {

local $ENV{MOJO_MODE} = 'foo';
cmp_deeply(
Test::Conch->new(config => { features => { audit => 0, no_db => 1 } })->app->log,
Test::Conch->new(config => { features => { no_db => 1 }, logging => { verbose => 0 } })->app->log,
all(
isa('Conch::Log'),
methods(
Expand All @@ -191,10 +191,7 @@ sub add_test_routes ($t) {

{
reset_log;
my $t = Test::Conch->new(config => {
logging => { handle => $log_fh },
features => { audit => 0 },
});
my $t = Test::Conch->new(config => { logging => { handle => $log_fh, verbose => 0 } });

cmp_deeply(
$t->app->log,
Expand Down Expand Up @@ -551,12 +548,7 @@ sub add_test_routes ($t) {

{
reset_log;
my $t = Test::Conch->new(
config => {
logging => { handle => $log_fh },
features => { audit => 1 },
},
);
my $t = Test::Conch->new(config => { logging => { handle => $log_fh, verbose => 1 } });

my %lines = add_test_routes($t);

Expand All @@ -580,7 +572,7 @@ sub add_test_routes ($t) {
},
msg => 'error line from controller',
},
'audit mode turns on trace mode',
'verbose mode turns on trace option',
);

cmp_deeply(
Expand Down Expand Up @@ -612,7 +604,7 @@ sub add_test_routes ($t) {
body => { error => 'something bad happened' },
},
},
'dispatch line for an error in audit mode includes both the request and response body',
'dispatch line for an error in verbose mode includes both the request and response body',
);

$t->post_ok('/_die?query_param=value0', json => { body_param => 'value1' })
Expand Down Expand Up @@ -673,7 +665,7 @@ sub add_test_routes ($t) {
),
},
},
'dispatch line for an uncaught exception in audit mode includes the full stack trace',
'dispatch line for an uncaught exception in verbose mode includes the full stack trace',
);

$t->post_ok('/login', json => { email => 'foo@example.com', password => 'PASSWORD' })
Expand Down Expand Up @@ -708,7 +700,7 @@ sub add_test_routes ($t) {
body => { error => 'Unauthorized' },
},
},
'dispatch line for /login error in audit mode does NOT contain the request body',
'dispatch line for /login error in verbose mode does NOT contain the request body',
);

my $user = $t->load_fixture('super_user');
Expand Down Expand Up @@ -742,7 +734,7 @@ sub add_test_routes ($t) {
# no body! that contains the JWT!!!
},
},
'dispatch line for /login success in audit mode',
'dispatch line for /login success in verbose mode',
);

$t->post_ok('/user/me/token', json => { name => 'my api token' })
Expand Down Expand Up @@ -778,7 +770,7 @@ sub add_test_routes ($t) {
# no body! that contains the api token!!!
},
},
'dispatch line for creating a token in audit mode does not contain the token string',
'dispatch line for creating a token in verbose mode does not contain the token string',
);

$t->post_ok('/user/me/password' => { Authorization => 'Bearer '.$t->tx->res->json->{token} },
Expand Down Expand Up @@ -814,7 +806,7 @@ sub add_test_routes ($t) {
body => '',
},
},
'dispatch line for changing password in audit mode does not contain the password',
'dispatch line for changing password in verbose mode does not contain the password',
);
}

Expand Down
4 changes: 2 additions & 2 deletions t/conch-rollbar.t
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ my $api_version_re = qr/^${ Test::Conch->API_VERSION_RE }$/;

my $t = Test::Conch->new(
config => {
features => { rollbar => 1, no_db => 1, audit => 0 },
features => { rollbar => 1, no_db => 1 },
rollbar => {
access_token => 'TOKEN',
environment => 'custom_environment',
error_match_header => { 'My-Buggy-Client' => qr/^1\.[0-9]$/ },
warn_payload_elements => 5,
warn_payload_size => 100,
},
logging => { handle => $log_fh },
logging => { handle => $log_fh, verbose => 0 },
},
pg => undef,
);
Expand Down

0 comments on commit de9856d

Please sign in to comment.