Skip to content

Loading…

Added SimpleDBHandler #113

Open
wants to merge 14 commits into from

4 participants

@claylo

I've added a SimpleDBHandler to log messages using a passed-in AmazonSDB object from the official Amazon SDK for PHP.

Given the size limitations (which are gracious, but still exist) of SimpleDB domains, I've taken the approach of mapping log channels-to-SimpleDB domains.

While SimpleDB has support for batch put operations, and the AmazonSDB class has support for sending items in parallel -- which, when combined, could conceivably allow for parallel batch puts -- I have kept this first commit simple, so that this handler's behavior mirrors that of most of the others.

If it winds up being popular and users request it, I'd be happy to contribute a further SimpleDBBatchHandler that dives into supporting those scenarios.

Meanwhile, I look forward to your feedback on this less complicated submission.

@Baachi

You should use mock object's for your test :)

@Baachi Baachi commented on an outdated diff
src/Monolog/Handler/SimpleDBHandler.php
((37 lines not shown))
+ * ));
+ * $sdb->set_region(AmazonSDB::REGION_CALIFORNIA);
+ *
+ * // recommended: create a SimpleDB Domain for each logging channel
+ * // $sdb->create_domain('my-logging-channel');
+ *
+ * $log->pushHandler(new SimpleDBHandler($sdb, Logger::WARNING));
+ *
+ * @author Clay Loveless <clay@php.net>
+ */
+class SimpleDBHandler extends AbstractProcessingHandler
+{
+ protected $sdb;
+ protected $origin_info;
+ protected $onEC2;
+ protected $ec2_metadata_keys = array(
@Baachi
Baachi added a note

Please remove the underscores and use camelize :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Baachi Baachi commented on an outdated diff
src/Monolog/Handler/SimpleDBHandler.php
((103 lines not shown))
+ *
+ * Usage example:
+ *
+ * $log = new Logger('my-logging-channel');
+ * $sdb = new AmazonSDB(array(
+ * 'certificate_authority' => __DIR__.'/../vendor/amazonwebservices/aws-sdk-for-php/lib/requestcore/cacert.pem'
+ * 'key' => 'AWS Access Key',
+ * 'secret' => 'AWS Secret Key'
+ * ));
+ * // $sdb->set_region(AmazonSDB::REGION_CALIFORNIA);
+ *
+ * $sdb_handler = new SimpleDBHandler($sdb, Logger::WARNING, true, array('hostname', 'ami-id', 'kernel-id'));
+ *
+ * $log->pushHandler($sdb_handler);
+ *
+ * @return void
@Baachi
Baachi added a note

@return void is not needed. You can remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Baachi Baachi commented on an outdated diff
src/Monolog/Handler/SimpleDBHandler.php
((130 lines not shown))
+ if (is_array($this->onEC2)) {
+ $metadata_keys = $this->onEC2;
+ } else {
+ $metadata_keys = $this->ec2_metadata_keys;
+ }
+
+ // since socket is already open, use it
+ foreach ($metadata_keys as $metakey) {
+ $req = "GET /latest/meta-data/{$metakey} HTTP/1.0";
+ fwrite($fp, $req);
+ $origin_info[$metakey] = stream_get_contents($fp);
+ }
+ fclose($fp);
+ } else {
+ // log a warning in hopes they'll notice and fix their settings
+ trigger_error(
@Baachi
Baachi added a note

Why you use trigger_error instead of throwing a exception?

@claylo
claylo added a note

Because it's not an exceptional scenario, but worth mentioning. A suggestion about configuration doesn't rank as a runtime-halting event.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stof stof commented on an outdated diff
src/Monolog/Handler/SimpleDBHandler.php
((153 lines not shown))
+ 'hostname' => php_uname('n')
+ );
+ }
+
+ $this->origin_info = $origin_info;
+ }
+
+ /**
+ * Buffer writes so SimpleDB BatchPutAttributes can be used
+ *
+ */
+ protected function write(array $record)
+ {
+ if ($record['level'] < $this->level) {
+ return false;
+ }
@stof
stof added a note

This is useless as it is already done by the parent class before calling write

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stof stof commented on an outdated diff
src/Monolog/Handler/SimpleDBHandler.php
((156 lines not shown))
+
+ $this->origin_info = $origin_info;
+ }
+
+ /**
+ * Buffer writes so SimpleDB BatchPutAttributes can be used
+ *
+ */
+ protected function write(array $record)
+ {
+ if ($record['level'] < $this->level) {
+ return false;
+ }
+
+ // send the domain create command for the channel, if necessary
+ if (! $this->skip_create) {
@stof
stof added a note

Please remove the extra space after !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stof stof commented on an outdated diff
src/Monolog/Handler/SimpleDBHandler.php
((172 lines not shown))
+ if (! in_array($record['channel'], $this->known_channels)) {
+ $this->sdb->create_domain($record['channel']);
+ $this->known_channels[] = $record['channel'];
+ }
+ }
+
+ $item_name = $this->getLogItemName();
+
+ $item = array(
+ 'datetime' => $record['datetime']->format(DATE_ISO8601),
+ 'level' => $record['level'],
+ 'message' => $record['formatted']['message']
+ );
+
+ // add context as first-class values
+ if (! empty($record['formatted']['context'])) {
@stof
stof added a note

this call is useless as the context will always be an array and you can iterate over an empty array

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stof stof commented on an outdated diff
src/Monolog/Handler/SimpleDBHandler.php
((175 lines not shown))
+ }
+ }
+
+ $item_name = $this->getLogItemName();
+
+ $item = array(
+ 'datetime' => $record['datetime']->format(DATE_ISO8601),
+ 'level' => $record['level'],
+ 'message' => $record['formatted']['message']
+ );
+
+ // add context as first-class values
+ if (! empty($record['formatted']['context'])) {
+ foreach ($record['formatted']['context'] as $context_key => $context_val) {
+ $item[$context_key] = $context_val;
+ }
@stof
stof added a note

Why not simply using array_merge ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@claylo

@Baachi Regarding mock object usage, I side with Martin Fowler on this, in that I take the classicist approach. In this case, testing the actual service is far less complicated than mocking the heinous AWS SDK. I choose test simplicity over strict mockist dogma every time. I discussed this via email with @Seldaek prior to submitting the pull request, and he had no objection.

@Baachi

I don't like the idea with the EC2 solution. I think the handler should not collect data, create a AmazonEC2Processor would be better. @stof @Seldaek

@Baachi

@claylo If Seldaek has no objection, it's okay :)

@claylo

@Baachi good idea on the AmazonEC2Processor. Hadn't dug into processors much when I started on the Handler. I'll make that change now.

@Baachi Baachi commented on an outdated diff
src/Monolog/Handler/SimpleDBHandler.php
((161 lines not shown))
+ * Buffer writes so SimpleDB BatchPutAttributes can be used
+ *
+ */
+ protected function write(array $record)
+ {
+ // send the domain create command for the channel, if necessary
+ if (!$this->skipCreate) {
+ if (!in_array($record['channel'], $this->knownChannels)) {
+ $this->sdb->create_domain($record['channel']);
+ $this->knownChannels[] = $record['channel'];
+ }
+ }
+
+ $item_name = $this->getLogItemName();
+
+ $item = array(
@Baachi
Baachi added a note

Move this code to a Formatter would be great.
@claylo What do you think?

@claylo
claylo added a note

This is unrelated to formatting ... this is the SimpleDB equivalent of "CREATE TABLE IF NOT EXISTS [tablename]".

@Baachi
Baachi added a note

Ah thank you for this explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Baachi

And AFAIK monolog follow's the Symfony2 styleguide

@claylo

@Baachi Apparently not strict adherence to Symfony2 styleguide ... from the "Naming conventions" section:

Use underscores for option, parameter names;
@Baachi

This line means the arguments in an array (or something else).
e.g.:

<?php
array(
    'some_host' => 'localhost',
    'some_port' =>  '5984'
);

All parameter's, functionnames, methodnames and variables are in camelCase:)

@claylo

Huh, guess you're right @Baachi. How strange that @fabpot wouldn't call class properties class properties when defining a style guide, since variables are something else entirely.

@Baachi

You are right, the definition's are inartfully expressed.

@Baachi

Tip: For you'r CS issues you can use the PHP-CS-Fixer. He will do the job for you :)

@claylo

Excellent tip! Thank you, wasn't aware of that project. Man, that @fabpot is a code firehose, for our collective benefit! :)

@stof stof commented on an outdated diff
src/Monolog/Handler/SimpleDBHandler.php
((47 lines not shown))
+{
+ protected $sdb;
+ protected $skipCreate;
+ protected $knownChannels = array();
+
+ /**
+ * Pass in the SimpleDB object, the level of logging to record, the bubble-factor
+ * and an optional $skip_create parameter to bypass attempts to create
+ * the SimpleDB domain.
+ *
+ * @param object $sdb AmazonSDB object
+ * @param integer $level Logging level
+ * @param boolean $bubble Whether the messages that are handled can bubble up the stack or not
+ * @param boolean $skip_create Whether or not to send the create_domain command for the log channel
+ */
+ public function __construct(\AmazonSDB $sdb, $level = Logger::DEBUG, $bubble = true, $skip_create = false)
@stof
stof added a note

The variable should be named skipCreate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stof stof commented on an outdated diff
src/Monolog/Handler/SimpleDBHandler.php
((69 lines not shown))
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ protected function write(array $record)
+ {
+ // send the domain create command for the channel, if necessary
+ if (!$this->skipCreate) {
+ if (!in_array($record['channel'], $this->knownChannels)) {
+ $this->sdb->create_domain($record['channel']);
+ $this->knownChannels[] = $record['channel'];
+ }
+ }
+
+ $item_name = $this->getLogItemName();
@stof
stof added a note

Please use camelCased names for the variables

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stof stof commented on an outdated diff
src/Monolog/Handler/SimpleDBHandler.php
((86 lines not shown))
+ $item = array(
+ 'datetime' => $record['datetime']->format(DATE_ISO8601),
+ 'level' => $record['level'],
+ 'message' => $record['formatted']['message']
+ );
+
+ // add 'context' and 'extra' as first-class values
+ $item = array_merge(
+ $item,
+ $record['formatted']['context'],
+ $record['formatted']['extra']
+ );
+
+ $this->sdb->put_attributes($record['channel'], $item_name, $item);
+
+ return false === $this->bubble;
@stof
stof added a note

wrtie does not need to return anything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@claylo

Is there anything else I can/should do on this contribution? I appreciate the input received thus far.

@Seldaek
Owner

@claylo I guess by now it's fine thanks, I just need to take some time to review monolog pull requests and push out a new version.

@Seldaek Seldaek commented on the diff
src/Monolog/Processor/AmazonEC2Processor.php
((48 lines not shown))
+ $this->baseUrl = $baseUrl;
+ }
+
+ /**
+ * @param array $record
+ * @return array
+ */
+ public function __invoke(array $record)
+ {
+ $metadata = array();
+
+ // should be an instant connection if on EC2, so timeout after
+ // one second to minimize delay for anyone using this processor
+ // mistakenly in a local dev environment.
+ $socketTimeout = ini_get('default_socket_timeout');
+ ini_set('default_socket_timeout', 1);
@Seldaek Owner
Seldaek added a note

using stream_context_create() with the right options for timeout would be better than ini_set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Seldaek Seldaek commented on the diff
src/Monolog/Processor/AmazonEC2Processor.php
((64 lines not shown))
+
+ // if we can't get hostname, we can't get any others either
+ $hostname = file_get_contents($this->baseUrl . '/latest/meta-data/hostname');
+ if ($hostname !== false) {
+ // don't get hostname twice
+ if (in_array('hostname', $this->metadataKeys)) {
+ $metadata['hostname'] = $hostname;
+ }
+ foreach ($this->metadataKeys as $metakey) {
+ if ($metakey == 'hostname') {
+ continue;
+ }
+ $url = $this->baseUrl . '/latest/meta-data/' . $metakey;
+ $metadata[$metakey] = file_get_contents($url);
+ }
+ }
@Seldaek Owner
Seldaek added a note

I think this whole block's result should be cached on the instance so that it's executed only once. It seems fairly expensive so doing it for every log record is gonna be quite wasteful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Seldaek Seldaek commented on the diff
src/Monolog/Processor/AmazonEC2Processor.php
((26 lines not shown))
+ 'hostname',
+ 'instance-id',
+ 'instance-type',
+ 'local-ipv4',
+ 'public-ipv4',
+ 'placement/availability-zone'
+ );
+
+ protected $baseUrl;
+
+ /**
+ * @param array $metadataKeys Array of metadata keys to record
+ * @param boolean $overwrite If TRUE, passed metadataKeys will not be merged
+ * @param string $baseUrl Used to set where the metadata should be queried
+ */
+ public function __construct(array $metadataKeys, $overwrite = false, $baseUrl = 'http://169.254.169.254')
@Seldaek Owner
Seldaek added a note

$metadataKeys could default to an empty array no?

@stof
stof added a note

what does the IP of the default value correspond to ?

@Seldaek Owner
Seldaek added a note

If you check the URL above http://docs.amazonwebservices.com/AWSEC2/latest/UserGuide/AESDG-chapter-instancedata.html it seems to be some magic metadata server's IP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Seldaek Seldaek commented on the diff
src/Monolog/Handler/SimpleDBHandler.php
((81 lines not shown))
+ }
+ }
+
+ $itemName = $this->getLogItemName();
+
+ $item = array(
+ 'datetime' => $record['datetime']->format(DATE_ISO8601),
+ 'level' => $record['level'],
+ 'message' => $record['formatted']['message']
+ );
+
+ // add 'context' and 'extra' as first-class values
+ $item = array_merge(
+ $item,
+ $record['formatted']['context'],
+ $record['formatted']['extra']
@Seldaek Owner
Seldaek added a note

Not sure this is a good idea, if the context contains a message or level key overwriting the original is not cool. Maybe best to keep them inside context/extra keys for safety.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Seldaek Seldaek commented on the diff
src/Monolog/Handler/SimpleDBHandler.php
((84 lines not shown))
+ $itemName = $this->getLogItemName();
+
+ $item = array(
+ 'datetime' => $record['datetime']->format(DATE_ISO8601),
+ 'level' => $record['level'],
+ 'message' => $record['formatted']['message']
+ );
+
+ // add 'context' and 'extra' as first-class values
+ $item = array_merge(
+ $item,
+ $record['formatted']['context'],
+ $record['formatted']['extra']
+ );
+
+ $this->sdb->put_attributes($record['channel'], $itemName, $item);
@Seldaek Owner
Seldaek added a note

I don't know how SimpleDB works in details, but as I understand here the channel is used as the "db" name sort of? Is this unique per account, or can you have multiple SDB "keys" per account? I ask because the channel in symfony2 for example is very simple/abstract stuff like "db", "request", "app". If you have two apps they would start colliding which may not be wanted. I think it would be best to allow the db to be specified in the constructor, and perhaps append the channel to it, or allow the db to contain %channel% in which case it is replaced by every record's channel?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Seldaek
Owner

@claylo ok really sorry for the delay but I went through the code thoroughly at last and gave feedback on a few things. I don't have much experience with SDB nor AWS in general so please correct me if needed.

@Baachi Baachi commented on the diff
src/Monolog/Handler/SimpleDBHandler.php
((69 lines not shown))
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ protected function write(array $record)
+ {
+ // send the domain create command for the channel, if necessary
+ if (!$this->skipCreate) {
+ if (!in_array($record['channel'], $this->knownChannels)) {
+ $this->sdb->create_domain($record['channel']);
+ $this->knownChannels[] = $record['channel'];
+ }
+ }
+
+ $itemName = $this->getLogItemName();
@Baachi
Baachi added a note

I'm not sure, but these lines should be moved to his own formatter.
@Seldaek what do you think?

@Seldaek Owner
Seldaek added a note

Yeah that would be one way to alleviate my concern about conflicts below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@claylo

Thanks for the feedback, guys. I'll take a look at these comments ASAP. Meanwhile, a lot needs to change anyway, since there's a whole new Amazon SDK for PHP out now, based on Guzzle ... so the SimpleDB implementation here should change to correspond with that as well.

@Seldaek
Owner

Yup I thought of that earlier but it seems their v2 doesn't support SimpleDB yet (cc @mtdowling)

@claylo

Indeed -- that's the pull request I've been working on over the holidays. :) So, it'll have SimpleDB support soon!

@Seldaek
Owner
@stof stof commented on the diff
tests/Monolog/Handler/SimpleDBHandlerTest.php
((26 lines not shown))
+ return;
+ }
+
+ static::$sdb = new \AmazonSDB(array(
+ 'key' => MONOLOG_AWS_SDB_KEY,
+ 'secret' => MONOLOG_AWS_SDB_SECRET,
+ 'certificate_authority' => __DIR__.'/../../../vendor/amazonwebservices/aws-sdk-for-php/lib/requestcore/cacert.pem'
+ ));
+ static::$sdb->set_region(MONOLOG_SDB_REGION);
+
+ static::$channel = uniqid('monolog_phpunit_events_');
+ }
+
+ public function setUp()
+ {
+ if (! class_exists('\AmazonSDB')) {
@stof
stof added a note

This is too late as setUpBeforeClass runs before setUp and you use the class there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stof stof commented on the diff
tests/Monolog/Handler/SimpleDBHandlerTest.php
((10 lines not shown))
+ */
+
+namespace Monolog\Handler;
+
+use Monolog\TestCase;
+use Monolog\Logger;
+use Monolog\Handler\SimpleDBHandler;
+
+class SimpleDBHandlerTest extends TestCase
+{
+ protected static $sdb;
+ protected static $channel;
+
+ public static function setUpBeforeClass()
+ {
+ if (! defined('MONOLOG_AWS_SDB_KEY')) {
@stof
stof added a note

you should check for all constants you use, not only one of them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stof

you should remove the .gitkeep file in the fixtures folder. It is useless as there is already a gitignore file in there anyway so the folder is already kept

@Seldaek Seldaek added the Feature label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
1 README.mdown
@@ -114,6 +114,7 @@ Handlers
- _AmqpHandler_: Logs records to an [amqp](http://www.amqp.org/) compatible
server. Requires the [php-amqp](http://pecl.php.net/package/amqp) extension (1.0+).
- _CubeHandler_: Logs records to a [Cube](http://square.github.com/cube/) server.
+- _SimpleDBHandler_: Logs records to a [SimpleDB](http://aws.amazon.com/simpledb) domain.
Wrappers / Special Handlers
---------------------------
View
4 composer.json
@@ -16,10 +16,12 @@
"php": ">=5.3.0"
},
"require-dev": {
- "mlehner/gelf-php": "1.0.*"
+ "mlehner/gelf-php": "1.0.*",
+ "amazonwebservices/aws-sdk-for-php": "1.5.*"
},
"suggest": {
"mlehner/gelf-php": "Allow sending log messages to a GrayLog2 server",
+ "amazonwebservices/aws-sdk-for-php": "Allow sending log messages to Amazon SimpleDB",
"ext-amqp": "Allow sending log messages to an AMQP server (1.0+ required)",
"ext-mongo": "Allow sending log messages to a MongoDB server"
},
View
144 src/Monolog/Handler/SimpleDBHandler.php
@@ -0,0 +1,144 @@
+<?php
+
+/*
+ * This file is part of the Monolog package.
+ *
+ * (c) Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Monolog\Handler;
+
+use Monolog\Logger;
+use Monolog\Formatter\NormalizerFormatter;
+use Monolog\Handler\AbstractProcessingHandler;
+
+/**
+ * Logs to a SimpleDB Domain, using Amazon's AWS SDK for PHP.
+ *
+ * You'll probably want to create the domain in advance, since
+ * creating new domains can take up to 10 seconds.
+ *
+ * However, creating SimpleDB domains is an idempotent operation; running it
+ * multiple times using the same domain name will not result in an error
+ * response.
+ *
+ * Usage example:
+ *
+ * $log = new Logger('my-logging-channel');
+ * $sdb = new AmazonSDB(array(
+ * // bundled, but not used by default? Weird, but true.
+ * 'certificate_authority' => __DIR__.'/../vendor/amazonwebservices/aws-sdk-for-php/lib/requestcore/cacert.pem'
+ * 'key' => 'AWS Access Key',
+ * 'secret' => 'AWS Secret Key'
+ * ));
+ * $sdb->set_region(AmazonSDB::REGION_CALIFORNIA);
+ *
+ * // recommended: create a SimpleDB Domain for each logging channel
+ * // $sdb->create_domain('my-logging-channel');
+ *
+ * $log->pushHandler(new SimpleDBHandler($sdb, Logger::WARNING));
+ *
+ * @author Clay Loveless <clay@php.net>
+ */
+class SimpleDBHandler extends AbstractProcessingHandler
+{
+ protected $sdb;
+ protected $skipCreate;
+ protected $knownChannels = array();
+
+ /**
+ * Pass in the SimpleDB object, the level of logging to record, the bubble-factor
+ * and an optional $skip_create parameter to bypass attempts to create
+ * the SimpleDB domain.
+ *
+ * @param object $sdb AmazonSDB object
+ * @param integer $level Logging level
+ * @param boolean $bubble Whether the messages that are handled can bubble up the stack or not
+ * @param boolean $skipCreate Whether or not to send the create_domain command for the log channel
+ */
+ public function __construct(\AmazonSDB $sdb, $level = Logger::DEBUG, $bubble = true, $skipCreate = false)
+ {
+ $this->sdb = $sdb;
+
+ $this->skipCreate = (bool) $skipCreate;
+
+ parent::__construct($level, $bubble);
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ protected function write(array $record)
+ {
+ // send the domain create command for the channel, if necessary
+ if (!$this->skipCreate) {
+ if (!in_array($record['channel'], $this->knownChannels)) {
+ $this->sdb->create_domain($record['channel']);
+ $this->knownChannels[] = $record['channel'];
+ }
+ }
+
+ $itemName = $this->getLogItemName();
@Baachi
Baachi added a note

I'm not sure, but these lines should be moved to his own formatter.
@Seldaek what do you think?

@Seldaek Owner
Seldaek added a note

Yeah that would be one way to alleviate my concern about conflicts below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ $item = array(
+ 'datetime' => $record['datetime']->format(DATE_ISO8601),
+ 'level' => $record['level'],
+ 'message' => $record['formatted']['message']
+ );
+
+ // add 'context' and 'extra' as first-class values
+ $item = array_merge(
+ $item,
+ $record['formatted']['context'],
+ $record['formatted']['extra']
@Seldaek Owner
Seldaek added a note

Not sure this is a good idea, if the context contains a message or level key overwriting the original is not cool. Maybe best to keep them inside context/extra keys for safety.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ );
+
+ $this->sdb->put_attributes($record['channel'], $itemName, $item);
@Seldaek Owner
Seldaek added a note

I don't know how SimpleDB works in details, but as I understand here the channel is used as the "db" name sort of? Is this unique per account, or can you have multiple SDB "keys" per account? I ask because the channel in symfony2 for example is very simple/abstract stuff like "db", "request", "app". If you have two apps they would start colliding which may not be wanted. I think it would be best to allow the db to be specified in the constructor, and perhaps append the channel to it, or allow the db to contain %channel% in which case it is replaced by every record's channel?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+
+ /**
+ * Create a UUID without relying on ext/uuid
+ *
+ * @return string
+ */
+ protected function getLogItemName()
+ {
+ /**
+ * Thanks to Andrew Moore.
+ * @see http://www.php.net/manual/en/function.uniqid.php#94959
+ */
+
+ return sprintf('%04x%04x-%04x-%04x-%04x-%04x%04x%04x',
+
+ // 32 bits for "time_low"
+ mt_rand(0, 0xffff), mt_rand(0, 0xffff),
+
+ // 16 bits for "time_mid"
+ mt_rand(0, 0xffff),
+
+ // 16 bits for "time_hi_and_version"
+ // four most significant bits holds version number 4
+ mt_rand(0, 0x0fff) | 0x4000,
+
+ // 16 bits, 8 bits for "clk_seq_hi_res",
+ // 8 bits for "clk_seq_low",
+ // two most significant bits holds zero and one for variant DCE1.1
+ mt_rand(0, 0x3fff) | 0x8000,
+
+ // 48 bits for "node"
+ mt_rand(0, 0xffff), mt_rand(0, 0xffff), mt_rand(0, 0xffff)
+ );
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ protected function getDefaultFormatter()
+ {
+ return new NormalizerFormatter();
+ }
+
+}
View
91 src/Monolog/Processor/AmazonEC2Processor.php
@@ -0,0 +1,91 @@
+<?php
+
+/*
+ * This file is part of the Monolog package.
+ *
+ * (c) Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Monolog\Processor;
+
+/**
+ * Injects several EC2 instance properties in all records
+ *
+ * @author Clay Loveless <clay@php.net>
+ */
+class AmazonEC2Processor
+{
+ /**
+ * @see http://docs.amazonwebservices.com/AWSEC2/latest/UserGuide/AESDG-chapter-instancedata.html
+ */
+ protected $metadataKeys = array(
+ 'ami-id',
+ 'hostname',
+ 'instance-id',
+ 'instance-type',
+ 'local-ipv4',
+ 'public-ipv4',
+ 'placement/availability-zone'
+ );
+
+ protected $baseUrl;
+
+ /**
+ * @param array $metadataKeys Array of metadata keys to record
+ * @param boolean $overwrite If TRUE, passed metadataKeys will not be merged
+ * @param string $baseUrl Used to set where the metadata should be queried
+ */
+ public function __construct(array $metadataKeys, $overwrite = false, $baseUrl = 'http://169.254.169.254')
@Seldaek Owner
Seldaek added a note

$metadataKeys could default to an empty array no?

@stof
stof added a note

what does the IP of the default value correspond to ?

@Seldaek Owner
Seldaek added a note

If you check the URL above http://docs.amazonwebservices.com/AWSEC2/latest/UserGuide/AESDG-chapter-instancedata.html it seems to be some magic metadata server's IP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ if ($overwrite) {
+ $this->metadataKeys = $metadataKeys;
+ } else {
+ $this->metadataKeys = array_merge($this->metadataKeys, $metadataKeys);
+ }
+ $this->baseUrl = $baseUrl;
+ }
+
+ /**
+ * @param array $record
+ * @return array
+ */
+ public function __invoke(array $record)
+ {
+ $metadata = array();
+
+ // should be an instant connection if on EC2, so timeout after
+ // one second to minimize delay for anyone using this processor
+ // mistakenly in a local dev environment.
+ $socketTimeout = ini_get('default_socket_timeout');
+ ini_set('default_socket_timeout', 1);
@Seldaek Owner
Seldaek added a note

using stream_context_create() with the right options for timeout would be better than ini_set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ // if we can't get hostname, we can't get any others either
+ $hostname = file_get_contents($this->baseUrl . '/latest/meta-data/hostname');
+ if ($hostname !== false) {
+ // don't get hostname twice
+ if (in_array('hostname', $this->metadataKeys)) {
+ $metadata['hostname'] = $hostname;
+ }
+ foreach ($this->metadataKeys as $metakey) {
+ if ($metakey == 'hostname') {
+ continue;
+ }
+ $url = $this->baseUrl . '/latest/meta-data/' . $metakey;
+ $metadata[$metakey] = file_get_contents($url);
+ }
+ }
@Seldaek Owner
Seldaek added a note

I think this whole block's result should be cached on the instance so that it's executed only once. It seems fairly expensive so doing it for every log record is gonna be quite wasteful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ // restore socket settings
+ ini_set('default_socket_timeout', $socketTimeout);
+
+ $record['extra'] = array_merge(
+ $record['extra'],
+ $metadata
+ );
+
+ return $record;
+ }
+}
View
36 src/Monolog/Processor/HostnameProcessor.php
@@ -0,0 +1,36 @@
+<?php
+
+/*
+ * This file is part of the Monolog package.
+ *
+ * (c) Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Monolog\Processor;
+
+/**
+ * Injects environment's hostname in all records
+ *
+ * @author Clay Loveless <clay@php.net>
+ */
+class HostnameProcessor
+{
+ /**
+ * @param array $record
+ * @return array
+ */
+ public function __invoke(array $record)
+ {
+ $record['extra'] = array_merge(
+ $record['extra'],
+ array(
+ 'hostname' => php_uname('n'),
+ )
+ );
+
+ return $record;
+ }
+}
View
118 tests/Monolog/Handler/SimpleDBHandlerTest.php
@@ -0,0 +1,118 @@
+<?php
+
+/*
+ * This file is part of the Monolog package.
+ *
+ * (c) Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Monolog\Handler;
+
+use Monolog\TestCase;
+use Monolog\Logger;
+use Monolog\Handler\SimpleDBHandler;
+
+class SimpleDBHandlerTest extends TestCase
+{
+ protected static $sdb;
+ protected static $channel;
+
+ public static function setUpBeforeClass()
+ {
+ if (! defined('MONOLOG_AWS_SDB_KEY')) {
@stof
stof added a note

you should check for all constants you use, not only one of them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ return;
+ }
+
+ static::$sdb = new \AmazonSDB(array(
+ 'key' => MONOLOG_AWS_SDB_KEY,
+ 'secret' => MONOLOG_AWS_SDB_SECRET,
+ 'certificate_authority' => __DIR__.'/../../../vendor/amazonwebservices/aws-sdk-for-php/lib/requestcore/cacert.pem'
+ ));
+ static::$sdb->set_region(MONOLOG_SDB_REGION);
+
+ static::$channel = uniqid('monolog_phpunit_events_');
+ }
+
+ public function setUp()
+ {
+ if (! class_exists('\AmazonSDB')) {
@stof
stof added a note

This is too late as setUpBeforeClass runs before setUp and you use the class there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $this->markTestSkipped('amazonwebservices/aws-sdk-for-php not installed');
+ }
+ if (! defined('MONOLOG_AWS_SDB_KEY')) {
+ $this->markTestSkipped('SimpleDB constants are not set in phpunit.xml');
+ }
+
+ if (empty(static::$channel)) {
+ $this->markTestSkipped('Gave up waiting on test domain creation');
+ }
+
+ // do we need to create the test domain?
+ $domains = static::$sdb->get_domain_list();
+
+ if (! in_array(static::$channel, $domains)) {
+ $result = static::$sdb->create_domain(static::$channel);
+
+ $channelFound = false;
+ $waitMax = 20;
+ $wait = 0;
+ while (! $channelFound) {
+ // wait awhile
+ sleep(1);
+ $wait++;
+ $domains = static::$sdb->get_domain_list();
+ if (in_array(static::$channel, $domains)) {
+ $channelFound = true;
+ }
+ if ($wait == $waitMax) {
+ static::$channel = null;
+ $this->markTestSkipped("Waited {$wait_max} seconds for test domain creation");
+ break;
+ }
+ }
+ }
+
+ }
+
+ public function testConstruct()
+ {
+ $handler = $this->getHandler();
+ $this->assertInstanceOf('Monolog\Handler\SimpleDBHandler', $handler);
+ }
+
+ public function testDebug()
+ {
+ $handler = $this->getHandler();
+
+ $record = $this->getRecord(Logger::DEBUG, "A testDebug message");
+ $record['channel'] = static::$channel;
+ $handler->handle($record);
+
+ // account for eventual consistency
+ sleep(3);
+
+ $result = static::$sdb->select("SELECT * FROM ".static::$channel." WHERE level='".Logger::DEBUG."'");
+ $msg = null;
+ foreach ($result->body->SelectResult->Item->Attribute as $att) {
+ if ($att->Name == 'message') {
+ $msg = (string) $att->Value;
+ }
+ }
+
+ $this->assertSame('A testDebug message', $msg);
+ }
+
+ protected function getHandler()
+ {
+ return new SimpleDBHandler(static::$sdb, Logger::DEBUG, true, false);
+ }
+
+ public static function tearDownAfterClass()
+ {
+ if (! empty(static::$sdb)) {
+ static::$sdb->delete_domain(static::$channel);
+ }
+ }
+}
View
85 tests/Monolog/Processor/AmazonEC2ProcessorTest.php
@@ -0,0 +1,85 @@
+<?php
+
+/*
+ * This file is part of the Monolog package.
+ *
+ * (c) Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Monolog\Processor;
+
+use Monolog\TestCase;
+
+class AmazonEC2ProcessorTest extends TestCase
+{
+ protected static $metadataFixture = array(
+ 'hostname' => 'ip-10-170-138-51.us-west-1.compute.internal',
+ 'ami-id' => 'ami-e78cd3a1',
+ 'instance-id' => 'i-1abfb30a',
+ 'instance-type' => 't1.micro',
+ 'local-ipv4' => '10.170.138.51',
+ 'public-ipv4' => '204.236.140.81',
+ 'placement/availability-zone' => 'us-west-1c'
+ );
+ protected static $tmpdir;
+
+ public static function setUpBeforeClass()
+ {
+ self::$tmpdir = __DIR__ . '/Fixtures/' . uniqid('AmazonEC2ProcessorTest_');
+ mkdir(self::$tmpdir . '/latest/meta-data/placement', 0777, true);
+ foreach (self::$metadataFixture as $key => $val) {
+ file_put_contents(self::$tmpdir . '/latest/meta-data/' . $key, $val);
+ }
+ }
+
+ /**
+ * @covers Monolog\Processor\AmazonEC2Processor
+ */
+ public function testProcessor()
+ {
+ $baseUrl = "file://".self::$tmpdir;
+ $processor = new AmazonEC2Processor(array(), false, $baseUrl);
+ $record = $processor($this->getRecord());
+ $this->assertArrayHasKey('hostname', $record['extra']);
+ $this->assertSame('t1.micro', $record['extra']['instance-type']);
+ }
+
+ public function testProcessorCanOverrideDefaultMetadataKeys()
+ {
+ $baseUrl = "file://".self::$tmpdir;
+ $processor = new AmazonEC2Processor(array('public-ipv4'), true, $baseUrl);
+ $record = $processor($this->getRecord());
+ $this->assertArrayNotHasKey('hostname', $record['extra']);
+ $this->assertArrayHasKey('public-ipv4', $record['extra']);
+ $this->assertSame('204.236.140.81', $record['extra']['public-ipv4']);
+ }
+
+ public function testProcessorCanMergeWithDefaultMetadataKeys()
+ {
+ $mac = '12:31:40:00:85:CA';
+ self::$metadataFixture['mac'] = $mac;
+ file_put_contents(self::$tmpdir . '/latest/meta-data/mac', $mac);
+
+ $baseUrl = "file://".self::$tmpdir;
+ $processor = new AmazonEC2Processor(array('mac'), false, $baseUrl);
+ $record = $processor($this->getRecord());
+ $this->assertArrayHasKey('hostname', $record['extra']);
+ $this->assertArrayHasKey('public-ipv4', $record['extra']);
+ $this->assertArrayHasKey('mac', $record['extra']);
+ $this->assertSame($mac, $record['extra']['mac']);
+ }
+
+ public static function tearDownAfterClass()
+ {
+ foreach (self::$metadataFixture as $key => $val) {
+ unlink(self::$tmpdir . '/latest/meta-data/' . $key);
+ }
+ rmdir(self::$tmpdir . '/latest/meta-data/placement');
+ rmdir(self::$tmpdir . '/latest/meta-data');
+ rmdir(self::$tmpdir . '/latest');
+ rmdir(self::$tmpdir);
+ }
+}
View
1 tests/Monolog/Processor/Fixtures/.gitignore
@@ -0,0 +1 @@
+AmazonEC2ProcessorTest_*/
View
0 tests/Monolog/Processor/Fixtures/.gitkeep
No changes.
View
30 tests/Monolog/Processor/HostnameProcessorTest.php
@@ -0,0 +1,30 @@
+<?php
+
+/*
+ * This file is part of the Monolog package.
+ *
+ * (c) Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Monolog\Processor;
+
+use Monolog\TestCase;
+
+class HostnameProcessorTest extends TestCase
+{
+ /**
+ * @covers Monolog\Processor\HostnameProcessor
+ */
+ public function testProcessor()
+ {
+ $processor = new HostnameProcessor();
+ $record = $processor($this->getRecord());
+ $this->assertArrayHasKey('hostname', $record['extra']);
+
+ $hostname = php_uname('n');
+ $this->assertSame($hostname, $record['extra']['hostname']);
+ }
+}
Something went wrong with that request. Please try again.