I've seen that the tag support has currently been implemented only for the InfluxDB collector, so I'd like to try giving my contributon by adding it to the StatsD collector as well.

When using this library in Symfony 4, you get the following error

Fatal error: Uncaught Error: Class 'Symfony\Component\DependencyInjection\DefinitionDecorator'
not found in vendor/beberlei/metrics/src/Beberlei/Bundle/MetricsBundle/DependencyInjection/BeberleiMetricsExtension.php
on line 39

In the Symfony 4.0 upgrade guide there is a note:

The DefinitionDecorator class has been removed. Use the ChildDefinition class instead.

Problem: The alternative (ChildDefinition) was introduced in Symfony 3.3, but this package appears to support earlier versions.

So we should either up the minimal requirement to 3.3 (or 4.0?), or do some kind of class_exists hack. Preferences?

The context logger makes use of the context array replacement feature defined in PSR-3. See https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-3-logger-interface.md#12-message for an example.

If you use monolog you should add the PsrLogMessageProcessor to your logger to enable replacement of the variables.

First, great library, I just thought about implementing something similar to prevent vendor lock-in. For now we will use datadog for our metric needs. StatsD (Datadog) supports adding arbitrary tags to metrics. Could that be added on the Collector API level with the default that other Collectors would not support tags? Or perhaps there are even more collectors that also support tags and could be enhanced to support tags. If we would make the parameter tags optional with an empty default value it would also be backwards compatible for all users of the library. Only the collectors would need to be modified to account for the new parameter. What do you think? (I would gladly implement this if there is a chance to include this, otherwise I will probably create my own fork)

I was just looking at the library and it struck me odd why there is no destructor defined that would flush. Is that on purpose and if so whats the rationale behind it? If there is nothing against it I'd gladly add it in.

Code to reproduce:

<?php

// Note: nothing listen on port 12345!!
$fp = fsockopen("udp://localhost", 12345, $errno, $errstr, 1.0);
var_dump($fp, $errno, $errstr); // Everything OK
fwrite($fp, "1"); // OK
fwrite($fp, "1"); // KO
fclose($fp);

echo "END";

Note:

No need to try catch, no exception can be trowed, and actually the code catch only Beberlei\Metrics\Exception which does not exist.

• [x] Use single quotes when possible
• [x] Improve readability of string constructs
• [ ] ~~Flags Graphite::push() as deprecated (shouldn't be part of the public interface)~~

Hello,

I just have a problem with the defer sending feature with the librato backend.

Let say I want to count a login event. So I put the $metric->increment('login'); code in the loginSuccess event. The controller then send the redirectResponse. The browser parse it and do the redirect and sadly I see that I am not yet logged.

I have found that the redirect is made before the session has been written (register_shutdown_function) because of the librato post that take too much time. So I wonder what is the best practice in this case. I guess I have to post my metrics before the redirect is sent to the browser.

What is your opinion on that ?

Fixes #58.

The bare minimum needed to allow Symfony 4, in a backwards compatible way. Basically fixes the point where it crashes.

Alternative, more proper fix can be found at #59.

Fixes #58.

Currently the (Symfony) class DefinitionDecorator is used, which has been renamed to ChildDefinition.

Problem at hand: this new class was only introduced in Symfony 3.3.

This is the most clean, but also a bit of an aggressive solution to this problem: require Symfony 3.3 (or 4.0).

• Use ChildDefinition over DefinitionDecorator
• Symfony 4 makes services private by default, so had to make some of them public in unit-tests.
• Was forced to use PHPUnit\Framework\TestCase over \PHPUnit_Framework_TestCase, since otherwise I was unable to install the later Symfony versions. This makes it compatible with PHPUnit 6, which is nice.
• Dropped php 5.4 and 5.5 support since Symfony 3 requires php >= 5.5 and PHPUnit does not include the namespaced TestCase class. Also they did not receive security updates >6 months by now.
• Dropped hhvm from the travis build since phpunit does not support hhvm: https://github.com/sebastianbergmann/phpunit/issues/2581

An alternative, less aggressive PR can be found at #60.

Hi there,

Thank you for this OS project which we use in different projects. I took some time to improve the overall lib public interface in order to clarify its integration.

Done

• [x] Added GaugeableCollector interface
• [x] Resets of InfluxDB::$data in InfluxDB::flush()
• [x] Comment consistency
• [x] Double-quote converted into single-quote when possible
• [x] Small code reformatting

Hope this PR will be a small step to better the project.

Thanks!

In order to prove out some basic metrics collection, I wanted to extend the InMemory class and customize the behavior of flush. This PR just makes the instance variables of InMemory protected so that I'm able to access them in my flush implementation.

Hey @lyrixx

here is an attempt to separate current InfluxDB integration and a new one using official package. This, if accepted, should replace #68 .

Unfortunately both packages use the same namespace InfluxDB so the new one isn't covered by tests. Any advice how to handle it correctly?

It looks like the example documentation on how to use the timing functionality isn't consistent with the expected data type and unit of measure:

README.md:

$start = microtime(true);
$diff  = microtime(true) - $start;
$collector->timing('foo.bar', $diff);

In this example, $diff will be a float representing the time delta value in microseconds, while the interface expects an int representing milliseconds:

Collector.php:

    /**
     * Records a timing.
     *
     * @param string $variable
     * @param int    $time     The duration of the timing in milliseconds
     */
    public function timing($variable, $time);

The flush method is required regardless of which collector type you're using. By adding this method to the GaugeableCollector interface, the collector type can be type-hinted on by the interface, rather than the implementation.

E.g. I can write

public function deliverMetrics(GaugeableCollector $collector) {
	$collector->gauge('metric', 1);
	$collector->flush();
}

instead of

public function deliverMetrics(StatsD $collector) {
	$collector->gauge('metric', 1);
	$collector->flush();
}