liuggio / statsd-php-client

Statsd (Object Oriented) client library for PHP
MIT License
115 stars 32 forks source link

Easily time functions with a closure. #43

Open dav-m85 opened 9 years ago

dav-m85 commented 9 years ago

An idea I find interesting, found on https://github.com/thephpleague/statsd:

$statsd->time('api.dbcall', function () {
    // this code execution will be timed and recorded in ms
});

Would be nice to implement here :)

tgr commented 9 years ago

Another nice pattern is

public function foo() {
    $scopedTimer = $statsd->scopedTimer('foo');
    // ...
}

and then the destructor of $scopedTimer will measure the time spent in the function. This can be convenient for functions with lots of exit points.

dav-m85 commented 9 years ago

To delegate the destructor the job of actually sending the timing results bugs me a little. Mainly for two reasons:

However I can see your point here. We could imagine for example enclosing foo() execution in the code of the issue proposal:

$statsd->time('api.dbcall', function () use (&$fooReturn) {
    $fooReturn = foo(); // your foo function
});

An idea to dig into.

tgr commented 9 years ago

It is a hack, but a very convenient one. E.g.

public function foo($statsd) {
    $startTime = microtime();
    doStuff();
    if (someFailureCondition()) {
         $statsd->time('foo', microtime() - $startTime);
         return false;
    }
    foreach (data() as $d) {
        try {
            doSomethingDangerousWith($d);
        } catch($e) {
            $statsd->time('foo', microtime() - $startTime);
            throw $e;
        }
    }
    $statsd->time('foo', microtime() - $startTime);
}

This kind of sucks. It is a lot nicer to say (although admittedly harder to understand at first glance):

public function foo($statsd) {
    $scopedTimer = $statsd->scopedTimer('foo');
    doStuff();
    if (someFailureCondition()) {
         return false;
    }
    foreach (data() as $d) {
        doSomethingDangerousWith($d);
    }
}

MediaWiki uses this pattern quite a bit (e.g. ScopedCallback); it's strange at first but works well.

liuggio commented 9 years ago

@dav-m85 I like your idea please when you have time add it... I think is better to place the flush in the deconstructor anyway with or without the @tgr idea