SentryLogWriter::factory() - Code Metrics - Inspection of "Userdata and exception context" - phptek/silverstripe-sentry - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#3)

unknown

created 2017-03-27 14:08 UTC

SentryLogWriter::factory() B

↳ Parent: SentryLogWriter

Complexity

Conditions	5
Paths	16

Size

Total Lines	28
Code Lines	15

Duplication

Lines	0
Ratio	0 %

Importance

Changes	1
Bugs	0	Features	0

Metric	Value
c	1
b	0
f	0
dl	0
loc	28
rs	8.439
cc	5
eloc	15
nc	16
nop	1

<?php


/**
 * Class: SentryLogWriter.
 *
 * @author  Russell Michell 2017 <[email protected]>
 * @package phptek/sentry
 */

namespace phptek\Sentry;

require_once THIRDPARTY_PATH . '/Zend/Log/Writer/Abstract.php';

/**
 * The SentryLogWriter class simply acts as a bridge between the configured Sentry 
 * adaptor and SilverStripe's {@link SS_Log}.
 * 
 * Usage in your project's _config.php for example (See README for examples).
 *  
 *    SS_Log::add_writer(\phptek\Sentry\SentryLogWriter::factory(), '<=');
 */

class SentryLogWriter extends \Zend_Log_Writer_Abstract
{
    
    /**
     * Stipulates what gets shown in the Sentry UI, should some metric not be
     * available for any reason.
     *
     * @const string
     */
    const SLW_NOOP = 'Unavailable';
    
    /**
     * A static constructor as per {@link Zend_Log_FactoryInterface}.
     * 
     * @param  array $config    An array of optional additional configuration for
     *                          passing custom information to Sentry. See the README for more detail.
     * @return SentryLogWriter
     */
    public static function factory($config = [])
    {
        $env = isset($config['env']) ? $config['env'] : null;
        $tags = isset($config['tags']) ? $config['tags'] : [];
        $extra = isset($config['extra']) ? $config['extra'] : [];
        $writer = \Injector::inst()->get('SentryLogWriter');

        // Set default environment
        if (is_null($env)) {
            $env = $writer->defaultEnv();
        }

        // Set all available user-data
        $userData = $writer->defaultUserData();

        // Set any available tags available in SS config
        $tags = array_merge($writer->defaultTags(), $tags);

        // Set any avalable additional (extra) data
        $extra = array_merge($writer->defaultExtra(), $extra);

        $writer->client->setData('env', $env);
        $writer->client->setData('user', $userData);
        $writer->client->setData('tags', $tags);
        $writer->client->setData('extra', $extra);

        return $writer;
    }

    /**
     * Used in unit tests.
     *
     * @return SentryClientAdaptor
     */
    public function getClient()
    {
        return $this->client;
class MyClass { }

$x = new MyClass();
$x->foo = true;
    }

    /**
     * Returns a default environment when one isn't passed to the factory()
     * method.
     *
     * @return string
     */
    public function defaultEnv()
    {
        return \Director::get_environment_type();
    }
    
    /**
     * Returns a default set of additional data specific to the user's part in
     * the request.
     * 
     * @param  Member $member
     * @return array
     */
    public function defaultUserData(\Member $member = null)
    {
        return [
            'IP-Address'    => $this->getIP(),
            'ID'            => $member ? $member->getField('ID') : self::SLW_NOOP,
            'Email'         => $member ? $member->getField('Email') : self::SLW_NOOP,
        ];
    }
    
    /**
     * Returns a default set of additional "tags" we wish to send to Sentry.
     * By default, Sentry reports on several mertrics, and we're already sending 
     * {@link Member} data. But there are additional data that would be useful
     * for debugging via the Sentry UI.
     *
     * These data can augment that which is sent to Sentry at setup
     * time in _config.php. See the README for more detail.
     *
     * N.b. Tags can be used to group messages within the Sentry UI itself, so there
     * should only be "static" data being sent, not something that can drastically
     * or minutely change, such as memory usage for example.
     * 
     * @return array
     */
    public function defaultTags()
    {
        return [
            'Request-Method'=> $this->getReqMethod(),
            'Request-Type'  => $this->getRequestType(),
            'SAPI'          => $this->getSAPI(),
            'SS-Version'    => $this->getPackageInfo('silverstripe/framework')
        ];
    }

    /**
     * Returns a default set of extra data to show upon selecting a message for
     * analysis in the Sentry UI. This can augment the data sent to Sentry at setup
     * time in _config.php as well as at runtime when calling SS_Log itself.
     * See the README for more detail.
     *
     * @return array
     */
    public function defaultExtra()
    {
        return [
            'Peak-Memory'   => $this->getPeakMemory()
        ];
    }
    
    /**
     * _write() forms the entry point into the physical sending of the error. The 
     * sending itself is done by the current adaptor's `send()` method.
     * 
     * @param  array $event An array of data that is created in, and arrives here
     *                      via {@link SS_Log::log()} and {@link Zend_Log::log}.
     * @return void
     */
    protected function _write($event)
    {
        $message = $event['message']['errstr'];                             // From SS_Log::log()

        // The complete compliment of these data come via the Raven_Client::xxx_context() methods
        $data = [
            'timestamp' => strtotime($event['timestamp']),                  // From Zend_Log::log()

            'extra'     => isset($event['extra']) ? $event['extra'] : []    // From _config.php (Optional)
        ];

        // Collect user data when sending because session is not initialized in _config.php
        $this->client->setData('user', $this->defaultUserData(\Member::currentUser()));


        // Use given context if available
        if (!empty($event['message']['errcontext'])) {
            $bt = $event['message']['errcontext'];
            // Push current line into context
            array_unshift($bt, [
                'file' => $event['message']['errfile'],
                'line' => $event['message']['errline'],
                'function' => '',
                'class' => '',
                'type' => '',
                'args' => [],
            ]);
        } else {
            $bt = debug_backtrace();
        }
        $trace = \SS_Backtrace::filter_backtrace($bt, ['SentryLogWriter->_write', 'phptek\Sentry\SentryLogWriter->_write']);

        $this->client->send($message, [], $data, $trace);
    }
    
    /**
     * Return the version of $pkg taken from composer.lock.
     * 
     * @param  string $pkg e.g. "silverstripe/framework"
     * @return string
     */
    public function getPackageInfo($pkg)
    {
        $lockFileJSON = BASE_PATH . '/composer.lock';

        if (!file_exists($lockFileJSON) || !is_readable($lockFileJSON)) {
            return self::SLW_NOOP;
        }

        $lockFileData = json_decode(file_get_contents($lockFileJSON), true);

        foreach ($lockFileData['packages'] as $package) {
            if ($package['name'] === $pkg) {
                return $package['version'];
            }
        }
        
        return self::SLW_NOOP;
    }
    
    /**
     * Return the IP address of the relevant request.
     * 
     * @return string
     */
    public function getIP()
    {
        $req = \Injector::inst()->create('SS_HTTPRequest', $this->getReqMethod(), '');
        
        if ($ip = $req->getIP()) {
            return $ip;
        }
        
        return self::SLW_NOOP;
    }
    
    /**
     * What sort of request is this? (A harder question to answer than you might
     * think: http://stackoverflow.com/questions/6275363/what-is-the-correct-terminology-for-a-non-ajax-request)
     * 
     * @return string
     */
    public function getRequestType()
    {
        $isCLI = $this->getSAPI() !== 'cli';
        $isAjax = \Director::is_ajax();

        return $isCLI && $isAjax ? 'AJAX' : 'Non-Ajax';
    }
    
    /**
     * Return peak memory usage.
     * 
     * @return float
     */
    public function getPeakMemory()
    {
        $peak = memory_get_peak_usage(true) / 1024 / 1024;
        
        return (string) round($peak, 2) . 'Mb';
    }
    
    /**
     * Basic User-Agent check and return.
     * 
     * @return string
     */
    public function getUserAgent()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $ua = @$_SERVER['HTTP_USER_AGENT'];
        
        if (!empty($ua)) {
            return $ua;
        }
        
        return self::SLW_NOOP;
    }
    
    /**
     * Basic reuqest method check and return.
     * 
     * @return string
     */
    public function getReqMethod()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $method = @$_SERVER['REQUEST_METHOD'];
        
        if (!empty($method)) {
            return $method;
        }
        
        return self::SLW_NOOP;
    }
    
    /**
     * @return string
     */
    public function getSAPI()
    {
        return php_sapi_name();
    }
    
}


1			<?php
			0 ignored issues – show Coding Style Compatibility introduced 2017-02-25 04:51 UTC by Report Bug Copy Issue Report For compatibility and reusability of your code, PSR1 recommends that a file should introduce either new symbols (like classes, functions, etc.) or have side-effects (like outputting something, or including other files), but not both at the same time. The first symbol is defined on line 23 and the first side effect is on line 12. The PSR-1: Basic Coding Standard recommends that a file should either introduce new symbols, that is classes, functions, constants or similar, or have side effects. Side effects are anything that executes logic, like for example printing output, changing ini settings or writing to a file. The idea behind this recommendation is that merely auto-loading a class should not change the state of an application. It also promotes a cleaner style of programming and makes your code less prone to errors, because the logic is not spread out all over the place. To learn more about the PSR-1, please see the PHP-FIG site on the PSR-1. Loading history...
2
3			/**
4			* Class: SentryLogWriter.
5			*
6			* @author Russell Michell 2017 <[email protected]>
7			* @package phptek/sentry
8			*/
9
10			namespace phptek\Sentry;
11
12			require_once THIRDPARTY_PATH . '/Zend/Log/Writer/Abstract.php';
13
14			/**
15			* The SentryLogWriter class simply acts as a bridge between the configured Sentry
16			* adaptor and SilverStripe's {@link SS_Log}.
17			*
18			* Usage in your project's _config.php for example (See README for examples).
19			*
20			* SS_Log::add_writer(\phptek\Sentry\SentryLogWriter::factory(), '<=');
21			*/
22
23			class SentryLogWriter extends \Zend_Log_Writer_Abstract
24			{
25
26			/**
27			* Stipulates what gets shown in the Sentry UI, should some metric not be
28			* available for any reason.
29			*
30			* @const string
31			*/
32			const SLW_NOOP = 'Unavailable';
33
34			/**
35			* A static constructor as per {@link Zend_Log_FactoryInterface}.
36			*
37			* @param array $config An array of optional additional configuration for
38			* passing custom information to Sentry. See the README for more detail.
39			* @return SentryLogWriter
40			*/
41			public static function factory($config = [])
42			{
43			$env = isset($config['env']) ? $config['env'] : null;
44			$tags = isset($config['tags']) ? $config['tags'] : [];
45			$extra = isset($config['extra']) ? $config['extra'] : [];
46			$writer = \Injector::inst()->get('SentryLogWriter');
47
48			// Set default environment
49			if (is_null($env)) {
50			$env = $writer->defaultEnv();
51			}
52
53			// Set all available user-data
54			$userData = $writer->defaultUserData();
55
56			// Set any available tags available in SS config
57			$tags = array_merge($writer->defaultTags(), $tags);
58
59			// Set any avalable additional (extra) data
60			$extra = array_merge($writer->defaultExtra(), $extra);
61
62			$writer->client->setData('env', $env);
63			$writer->client->setData('user', $userData);
64			$writer->client->setData('tags', $tags);
65			$writer->client->setData('extra', $extra);
66
67			return $writer;
68			}
69
70			/**
71			* Used in unit tests.
72			*
73			* @return SentryClientAdaptor
74			*/
75			public function getClient()
76			{
77			return $this->client;
			0 ignored issues – show Bug introduced 2017-03-02 19:12 UTC by Report Bug Copy Issue Report The property `client` does not exist. Did you maybe forget to declare it? In PHP it is possible to write to properties without declaring them. For example, the following is perfectly valid PHP code: class MyClass { } $x = new MyClass(); $x->foo = true; Generally, it is a good practice to explictly declare properties to avoid accidental typos and provide IDE auto-completion: class MyClass { public $foo; } $x = new MyClass(); $x->foo = true; Loading history...
78			}
79
80			/**
81			* Returns a default environment when one isn't passed to the factory()
82			* method.
83			*
84			* @return string
85			*/
86			public function defaultEnv()
87			{
88			return \Director::get_environment_type();
89			}
90
91			/**
92			* Returns a default set of additional data specific to the user's part in
93			* the request.
94			*
95			* @param Member $member
96			* @return array
97			*/
98			public function defaultUserData(\Member $member = null)
99			{
100			return [
101			'IP-Address' => $this->getIP(),
102			'ID' => $member ? $member->getField('ID') : self::SLW_NOOP,
103			'Email' => $member ? $member->getField('Email') : self::SLW_NOOP,
104			];
105			}
106
107			/**
108			* Returns a default set of additional "tags" we wish to send to Sentry.
109			* By default, Sentry reports on several mertrics, and we're already sending
110			* {@link Member} data. But there are additional data that would be useful
111			* for debugging via the Sentry UI.
112			*
113			* These data can augment that which is sent to Sentry at setup
114			* time in _config.php. See the README for more detail.
115			*
116			* N.b. Tags can be used to group messages within the Sentry UI itself, so there
117			* should only be "static" data being sent, not something that can drastically
118			* or minutely change, such as memory usage for example.
119			*
120			* @return array
121			*/
122			public function defaultTags()
123			{
124			return [
125			'Request-Method'=> $this->getReqMethod(),
126			'Request-Type' => $this->getRequestType(),
127			'SAPI' => $this->getSAPI(),
128			'SS-Version' => $this->getPackageInfo('silverstripe/framework')
129			];
130			}
131
132			/**
133			* Returns a default set of extra data to show upon selecting a message for
134			* analysis in the Sentry UI. This can augment the data sent to Sentry at setup
135			* time in _config.php as well as at runtime when calling SS_Log itself.
136			* See the README for more detail.
137			*
138			* @return array
139			*/
140			public function defaultExtra()
141			{
142			return [
143			'Peak-Memory' => $this->getPeakMemory()
144			];
145			}
146
147			/**
148			* _write() forms the entry point into the physical sending of the error. The
149			* sending itself is done by the current adaptor's `send()` method.
150			*
151			* @param array $event An array of data that is created in, and arrives here
152			* via {@link SS_Log::log()} and {@link Zend_Log::log}.
153			* @return void
154			*/
155			protected function _write($event)
156			{
157			$message = $event['message']['errstr']; // From SS_Log::log()
			0 ignored issues – show Unused Code Comprehensibility introduced 2017-02-25 04:51 UTC by Report Bug Copy Issue Report `38%` of this comment could be valid code. Did you maybe forget this after debugging? Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it. The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production. This check looks for comments that seem to be mostly valid code and reports them. Loading history...
158			// The complete compliment of these data come via the Raven_Client::xxx_context() methods
159			$data = [
160			'timestamp' => strtotime($event['timestamp']), // From Zend_Log::log()
			0 ignored issues – show Unused Code Comprehensibility introduced 2017-02-25 04:51 UTC by Report Bug Copy Issue Report `38%` of this comment could be valid code. Did you maybe forget this after debugging? Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it. The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production. This check looks for comments that seem to be mostly valid code and reports them. Loading history...
161			'extra' => isset($event['extra']) ? $event['extra'] : [] // From _config.php (Optional)
162			];
163
164			// Collect user data when sending because session is not initialized in _config.php
165			$this->client->setData('user', $this->defaultUserData(\Member::currentUser()));
			0 ignored issues – show Bug introduced 2017-03-27 14:11 UTC by Report Bug Copy Issue Report It seems like `\Member::currentUser()` targeting `Member::currentUser()` can also be of type `object<DataObject>`; however, `phptek\Sentry\SentryLogWriter::defaultUserData()` does only seem to accept `null\|object<Member>`, maybe add an additional type check? This check looks at variables that are passed out again to other methods. If the outgoing method call has stricter type requirements than the method itself, an issue is raised. An additional type check may prevent trouble. Loading history...
166
167			// Use given context if available
168			if (!empty($event['message']['errcontext'])) {
169			$bt = $event['message']['errcontext'];
170			// Push current line into context
171			array_unshift($bt, [
172			'file' => $event['message']['errfile'],
173			'line' => $event['message']['errline'],
174			'function' => '',
175			'class' => '',
176			'type' => '',
177			'args' => [],
178			]);
179			} else {
180			$bt = debug_backtrace();
181			}
182			$trace = \SS_Backtrace::filter_backtrace($bt, ['SentryLogWriter->_write', 'phptek\Sentry\SentryLogWriter->_write']);
183
184			$this->client->send($message, [], $data, $trace);
185			}
186
187			/**
188			* Return the version of $pkg taken from composer.lock.
189			*
190			* @param string $pkg e.g. "silverstripe/framework"
191			* @return string
192			*/
193			public function getPackageInfo($pkg)
194			{
195			$lockFileJSON = BASE_PATH . '/composer.lock';
196
197			if (!file_exists($lockFileJSON) \|\| !is_readable($lockFileJSON)) {
198			return self::SLW_NOOP;
199			}
200
201			$lockFileData = json_decode(file_get_contents($lockFileJSON), true);
202
203			foreach ($lockFileData['packages'] as $package) {
204			if ($package['name'] === $pkg) {
205			return $package['version'];
206			}
207			}
208
209			return self::SLW_NOOP;
210			}
211
212			/**
213			* Return the IP address of the relevant request.
214			*
215			* @return string
216			*/
217			public function getIP()
218			{
219			$req = \Injector::inst()->create('SS_HTTPRequest', $this->getReqMethod(), '');
220
221			if ($ip = $req->getIP()) {
222			return $ip;
223			}
224
225			return self::SLW_NOOP;
226			}
227
228			/**
229			* What sort of request is this? (A harder question to answer than you might
230			* think: http://stackoverflow.com/questions/6275363/what-is-the-correct-terminology-for-a-non-ajax-request)
231			*
232			* @return string
233			*/
234			public function getRequestType()
235			{
236			$isCLI = $this->getSAPI() !== 'cli';
237			$isAjax = \Director::is_ajax();
238
239			return $isCLI && $isAjax ? 'AJAX' : 'Non-Ajax';
240			}
241
242			/**
243			* Return peak memory usage.
244			*
245			* @return float
246			*/
247			public function getPeakMemory()
248			{
249			$peak = memory_get_peak_usage(true) / 1024 / 1024;
250
251			return (string) round($peak, 2) . 'Mb';
252			}
253
254			/**
255			* Basic User-Agent check and return.
256			*
257			* @return string
258			*/
259			public function getUserAgent()
			0 ignored issues – show Coding Style introduced 2017-02-25 04:51 UTC by Report Bug Copy Issue Report `getUserAgent` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
260			{
261			$ua = @$_SERVER['HTTP_USER_AGENT'];
262
263			if (!empty($ua)) {
264			return $ua;
265			}
266
267			return self::SLW_NOOP;
268			}
269
270			/**
271			* Basic reuqest method check and return.
272			*
273			* @return string
274			*/
275			public function getReqMethod()
			0 ignored issues – show Coding Style introduced 2017-02-25 04:51 UTC by Report Bug Copy Issue Report `getReqMethod` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
276			{
277			$method = @$_SERVER['REQUEST_METHOD'];
278
279			if (!empty($method)) {
280			return $method;
281			}
282
283			return self::SLW_NOOP;
284			}
285
286			/**
287			* @return string
288			*/
289			public function getSAPI()
290			{
291			return php_sapi_name();
292			}
293
294			}
295

phptek / silverstripe-sentry

Pull Request — master (#3)

SentryLogWriter::factory() B

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like