Skip to content

Commit

Permalink
Bug 956230: improve instrumentation of bugzilla's internals
Browse files Browse the repository at this point in the history
  • Loading branch information
globau committed Mar 4, 2014
1 parent 9193214 commit faf2fc5
Show file tree
Hide file tree
Showing 22 changed files with 919 additions and 102 deletions.
63 changes: 61 additions & 2 deletions Bugzilla.pm
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ use Bugzilla::Token;
use Bugzilla::User;
use Bugzilla::Util;

use Bugzilla::Metrics::Collector;
use Bugzilla::Metrics::Template;
use Bugzilla::Metrics::Memcached;

use File::Basename;
use File::Spec::Functions;
use DateTime::TimeZone;
Expand Down Expand Up @@ -127,6 +131,30 @@ sub init_page {

my $script = basename($0);

# BMO - init metrics collection if required
if (i_am_cgi() && $script eq 'show_bug.cgi') {
# we need to measure loading the params, so default to on
Bugzilla->metrics_enabled(1);
Bugzilla->metrics($script);
# we can now hit params to check if we really should be enabled.
# note - we can't use anything which uses templates or the database, as
# that would initialise those modules with metrics enabled.
if (!Bugzilla->params->{metrics_enabled}) {
Bugzilla->metrics_enabled(0);
}
else {
# to avoid generating massive amounts of data, we're only interested in
# a small subset of users
my $user_id = Bugzilla->cgi->cookie('Bugzilla_login');
if (!$user_id
|| !grep { $user_id == $_ }
split(/\s*,\s*/, Bugzilla->params->{metrics_user_ids}))
{
Bugzilla->metrics_enabled(0);
}
}
}

# Because of attachment_base, attachment.cgi handles this itself.
if ($script ne 'attachment.cgi') {
do_ssl_redirect_if_required();
Expand Down Expand Up @@ -195,7 +223,12 @@ sub init_page {
#####################################################################

sub template {
return $_[0]->request_cache->{template} ||= Bugzilla::Template->create();
# BMO - use metrics subclass if required
if (Bugzilla->metrics_enabled) {
return $_[0]->request_cache->{template} ||= Bugzilla::Metrics::Template->create();
} else {
return $_[0]->request_cache->{template} ||= Bugzilla::Template->create();
}
}

sub template_inner {
Expand Down Expand Up @@ -660,17 +693,43 @@ sub process_cache {
return $_process_cache;
}

# BMO - Instrumentation

sub metrics_enabled {
if (defined $_[1]) {
$_[0]->request_cache->{metrics_enabled} = $_[1];
delete $_[0]->request_cache->{metrics} unless $_[1];
}
else {
return $_[0]->request_cache->{metrics_enabled};
}
}

sub metrics {
return $_[0]->request_cache->{metrics} ||= Bugzilla::Metrics::Collector->new($_[1]);
}

# This is a memcached wrapper, which provides cross-process and cross-system
# caching.
sub memcached {
return $_[0]->process_cache->{memcached} ||= Bugzilla::Memcached->_new();
# BMO - use metrics subclass if required
if (Bugzilla->metrics_enabled) {
return $_[0]->request_cache->{memcached} ||= Bugzilla::Metrics::Memcached->_new();
} else {
return $_[0]->request_cache->{memcached} ||= Bugzilla::Memcached->_new();
}
}

# Private methods

# Per-process cleanup. Note that this is a plain subroutine, not a method,
# so we don't have $class available.
sub _cleanup {
# BMO - finalise and report on metrics
if (Bugzilla->metrics_enabled) {
Bugzilla->metrics->finish();
}

my $main = Bugzilla->request_cache->{dbh_main};
my $shadow = Bugzilla->request_cache->{dbh_shadow};
foreach my $dbh ($main, $shadow) {
Expand Down
31 changes: 31 additions & 0 deletions Bugzilla/Config/Advanced.pm
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,37 @@ use constant get_param_list => (
type => 't',
default => '',
},

{
name => 'metrics_enabled',
type => 'b',
default => 0
},
{
name => 'metrics_user_ids',
type => 't',
default => '3881,5038,5898,13647,20209,251051,373476,409787'
},
{
name => 'metrics_elasticsearch_server',
type => 't',
default => '127.0.0.1:9200'
},
{
name => 'metrics_elasticsearch_index',
type => 't',
default => 'bmo-metrics'
},
{
name => 'metrics_elasticsearch_type',
type => 't',
default => 'timings'
},
{
name => 'metrics_elasticsearch_ttl',
type => 't',
default => '1210000000' # 14 days
},
);

1;
8 changes: 8 additions & 0 deletions Bugzilla/DB.pm
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ use Bugzilla::Util;
use Bugzilla::Error;
use Bugzilla::DB::Schema;

use Bugzilla::Metrics::Mysql;

use List::Util qw(max);
use Storable qw(dclone);

Expand Down Expand Up @@ -148,6 +150,12 @@ sub _connect {
. " localconfig: " . $@);

# instantiate the correct DB specific module

# BMO - enable instrumentation of db calls
if (Bugzilla->metrics_enabled) {
$pkg_module = 'Bugzilla::Metrics::Mysql';
}

my $dbh = $pkg_module->new($params);

return $dbh;
Expand Down
1 change: 1 addition & 0 deletions Bugzilla/Install/Filesystem.pm
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ sub FILESYSTEM {
'jobqueue.pl' => { perms => OWNER_EXECUTE },
'migrate.pl' => { perms => OWNER_EXECUTE },
'sentry.pl' => { perms => OWNER_EXECUTE },
'metrics.pl' => { perms => OWNER_EXECUTE },
'install-module.pl' => { perms => OWNER_EXECUTE },

'Bugzilla.pm' => { perms => CGI_READ },
Expand Down
7 changes: 7 additions & 0 deletions Bugzilla/Install/Requirements.pm
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,13 @@ sub OPTIONAL_MODULES {
version => '0',
feature => ['memcached'],
},

# BMO - metrics
{
package => 'ElasticSearch',
module => 'ElasticSearch',
version => '0',
},
);

my $extra_modules = _get_extension_requirements('OPTIONAL_MODULES');
Expand Down
68 changes: 0 additions & 68 deletions Bugzilla/Instrument.pm

This file was deleted.

Loading

0 comments on commit faf2fc5

Please sign in to comment.