Added ElasticsearchLogstashHandler #1334
Conversation
|
Thanks @lyrixx, I am medium happy about this PR tbh, for a few reasons:
|
| parent::__construct($level, $bubble); | ||
| $this->endpoint = $endpoint; | ||
| $this->index = $index; | ||
| $this->client = $client ?: HttpClient::create(); |
There was a problem hiding this comment.
If we want to remove the dependency to the component, we can remove this call and make the $client argument mandatory. That will only require the http-client-contracts, which is an abstract dependency.
There was a problem hiding this comment.
we can also keep the argument optional, and throw an exception when the component is missing but not client has been provided, as done in https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/HttpClient/Psr18Client.php#L65
what does this mean? it's a standalone component.
if this is problematic, I'd suggest relying only on symfony/http-client-contracts, which is an abstraction |
|
HttpClient is not an hard dependency. This is the same as for the ElasticHandler or the LogstashHandler. |
|
The point is those other handlers were added ages ago, and removing them here would cause breakage for not much benefit, but for new handlers with third-party dependencies IMO the right way is to publish as a third-party package, with requires on monolog and on whichever dependency is needed. It lets Composer resolve everything which makes more sense really. @nicolas-grekas requiring the contracts package doesn't really solve the above, so IMO still best to just release as a separate package. That's why there is https://github.com/Seldaek/monolog/wiki/Third-Party-Packages - we can still add support for that third party package in the monolog-bundle for example. |
|
This means another maintainers need to deal with these per-handle packages. It may be easier to ask ppl to send PRs here: managing a package is very different from patching a class. I'd be more confident if you (monolog maintenance team) would deal with the releasing part. |
|
These handlers typically don't change much, so the package management aspect is very little.. We could do a monolog organization of some sort, but I don't really want to be managing a million handler packages for stuff that I don't know and don't care about, so IMO distributing this and having everyone maintaining their own handlers they are interested in makes more sense. I can see you may not agree with this seeing how the Symfony project handles this very differently, but that's life :) Anyway this handler in particular being dev-time only which really isn't the main use case for monolog.. If you feel it makes sense for symfony projects it could go in the monolog bridge or the monolog bundle there I guess. |
|
P.S; I can see how it'd be good to have a way to write to kibana directly tho, if you can make this support handleBatch and skip the http-client dependency then it might make more sense to me to have a production ready thing. |
Yes, I already proposed a "Contrib" Repository.
Kibana is only a "reader" on top of ES. I mean you can not write directly to kibana. You push logs to ES, and then you visualize them with Kibana
HandleBatch is really easy to do. Just 3/5 LOC to add. But I don't want to implement a new HTTP client here. Finally, http client is not added to the composer.json, it's only a soft deps, so I don't really see the issue. But I understand having another Handler means another Handler to maintain. |
ghost
mentioned this pull request
This PR was merged into the 4.4 branch. Discussion ---------- [Monolog] Added ElasticsearchLogstashHandler | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | This PR was initially [submitted on Monolog](Seldaek/monolog#1334). It has been refused , but Jordi suggested to add it to the symfony bridge. So here we go :) --- ATM, there are few options to push log to Elastic Stack in order to play with them: * install logstash and use a Gelf handler. It works but you have to install logstash and configure it. Not an easy task. More over, it need an extra PHP package * use the ES handler: It does not play well with context and extra: Kibana is not able to filter on nested object. And this handler is tightly coupled to the ElasticaFormatter formater. More over, it need an extra PHP package * use something to parse file logs. This is really a bad idea since it involves a parsing... More over a daemon is needed to do that (file beat / logstash / you name it) This is why I'm introducing a new Handler. * There is not need to install anything (expect ES, of course) * It play very well with Kibana, as it uses the Logstash format * It requires symfony/http-client, but in a modern PHP application (SF 4.3) this dependency is already present * It slow down a bit the application since it trigger an HTTP request for each logs. But symfony/http-client is non-blocking. If you want to use it in production, I recommend to wrap this handler in a buffer handler or a cross-finger handle to have only one HTTP call. --- Some performance consideration en a prod env with a buffer handler + this one * with push to ES: https://blackfire.io/profiles/f94ccf35-9f9d-4df1-bfc5-7fa75a535628/graph * with push to ES commented: https://blackfire.io/profiles/6b66bc18-6b90-4341-963f-797f7a7a689c/graph As you can see, as requests are made synchronously, there is no penalty on `AppKernel::Handler()` 😍! But the PHP worker has more work to do, and it's busy much more time (about X2) I explained everything in the PHP Doc Block --- This is what you can expect **out of the box**  Commits ------- 1587e9a [Monolog] Added ElasticsearchLogstashHandler
This PR was merged into the 4.4 branch. Discussion ---------- [Monolog] Added ElasticsearchLogstashHandler | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | This PR was initially [submitted on Monolog](Seldaek/monolog#1334). It has been refused , but Jordi suggested to add it to the symfony bridge. So here we go :) --- ATM, there are few options to push log to Elastic Stack in order to play with them: * install logstash and use a Gelf handler. It works but you have to install logstash and configure it. Not an easy task. More over, it need an extra PHP package * use the ES handler: It does not play well with context and extra: Kibana is not able to filter on nested object. And this handler is tightly coupled to the ElasticaFormatter formater. More over, it need an extra PHP package * use something to parse file logs. This is really a bad idea since it involves a parsing... More over a daemon is needed to do that (file beat / logstash / you name it) This is why I'm introducing a new Handler. * There is not need to install anything (expect ES, of course) * It play very well with Kibana, as it uses the Logstash format * It requires symfony/http-client, but in a modern PHP application (SF 4.3) this dependency is already present * It slow down a bit the application since it trigger an HTTP request for each logs. But symfony/http-client is non-blocking. If you want to use it in production, I recommend to wrap this handler in a buffer handler or a cross-finger handle to have only one HTTP call. --- Some performance consideration en a prod env with a buffer handler + this one * with push to ES: https://blackfire.io/profiles/f94ccf35-9f9d-4df1-bfc5-7fa75a535628/graph * with push to ES commented: https://blackfire.io/profiles/6b66bc18-6b90-4341-963f-797f7a7a689c/graph As you can see, as requests are made synchronously, there is no penalty on `AppKernel::Handler()` 😍! But the PHP worker has more work to do, and it's busy much more time (about X2) I explained everything in the PHP Doc Block --- This is what you can expect **out of the box**  Commits ------- 1587e9a4e2 [Monolog] Added ElasticsearchLogstashHandler
ATM, there are few options to push log to Elastic Stack in order to play with them:
This is why I'm introducing a new Handler.
Note: This handler is not aimed to be used in production
This is what you can expect out of the box
