Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Monolog] Added ElasticsearchLogstashHandler #32360

Merged
merged 1 commit into from
Aug 18, 2019

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Jul 4, 2019

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.
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

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
image

@nicolas-grekas
Copy link
Member

Note: This handler is not aimed to be used in productio

I'd suggest telling this in the docblock + explain how to do it in prod

@lyrixx
Copy link
Member Author

lyrixx commented Jul 4, 2019

I'd suggest telling this in the docblock + explain how to do it in prod

Actually, I could work a bit on it to make it available on production:
It it's used in combinaison with a buffer handler, or a finger cross handler, all logs could be sent by batch, and so only one HTTP call.

WDYT?

@lyrixx lyrixx force-pushed the monolog-elastic branch from 017cacb to efd4e54 Compare July 4, 2019 08:32
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your suggestion for a prod mode makes sense to me.

@lyrixx lyrixx force-pushed the monolog-elastic branch 2 times, most recently from 0a45301 to c30cf2d Compare July 8, 2019 13:29
@lyrixx
Copy link
Member Author

lyrixx commented Jul 8, 2019

  • I updated the PHPdoc.
  • I added blackfire profile to the PR description
  • I enhanced the main dockbloc
  • I added an entry to the readme
  • I changed the default host to 127.0.0.1:9200
  • I added a timeout to 1 sec

@lyrixx
Copy link
Member Author

lyrixx commented Jul 8, 2019

@nicolas-grekas How can I disable all exceptions in the http-client? Or at least catch them all? (But I think this is not do-able since I use the __destruct feature)

@lyrixx
Copy link
Member Author

lyrixx commented Jul 8, 2019

Hmm, I managed to do something. Could you look at the last commit? Thanks

@lyrixx
Copy link
Member Author

lyrixx commented Jul 16, 2019

I rebased this PR. It's now ready and green 💚

Copy link
Contributor

@noniagriconomie noniagriconomie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor doc review :)

edit : do you plan to add real doc, or a blog post to explain the config for dev/vs prod ?
i think it is a must have, to not degrade perf on prod :)

@lyrixx
Copy link
Member Author

lyrixx commented Jul 16, 2019

edit : do you plan to add real doc, or a blog post to explain the config for dev/vs prod ?
i think it is a must have, to not degrade perf on prod :)

Not really. There is already many blog post on internet about symfony + ELK

@lyrixx lyrixx force-pushed the monolog-elastic branch from 67c1c45 to 957d8c2 Compare July 16, 2019 16:43
@lyrixx
Copy link
Member Author

lyrixx commented Jul 16, 2019

I addressed your comments

@lyrixx lyrixx force-pushed the monolog-elastic branch 2 times, most recently from acf80a7 to 982f437 Compare July 17, 2019 09:10
@lyrixx
Copy link
Member Author

lyrixx commented Jul 17, 2019

my comment was more for configuring this new handler in the proper way inside a symfony dev|prod stack :)

This will be done in the monolog-bundle :)

@lyrixx lyrixx force-pushed the monolog-elastic branch from 0631133 to 8b1328c Compare July 25, 2019 09:07
@B-Galati
Copy link
Contributor

Do you plan to add some doc ?

@fabpot
Copy link
Member

fabpot commented Aug 18, 2019

Thank you @lyrixx.

fabpot added a commit that referenced this pull request Aug 18, 2019
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**
![image](https://user-images.githubusercontent.com/408368/59916122-9b7b7580-941e-11e9-9a22-f56bc1d1a288.png)

Commits
-------

1587e9a [Monolog] Added ElasticsearchLogstashHandler
@fabpot fabpot merged commit 1587e9a into symfony:4.4 Aug 18, 2019
@lyrixx lyrixx deleted the monolog-elastic branch August 18, 2019 21:23
@nicolas-grekas
Copy link
Member

@lyrixx could you please have a look at failures on branch master? It looks like the new class is not compatible with Monolog 2. Thanks.

@lyrixx
Copy link
Member Author

lyrixx commented Aug 19, 2019

👍 I will look at it. Thanks

@lyrixx
Copy link
Member Author

lyrixx commented Aug 19, 2019

#33241

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 2019
@DavidBadura
Copy link
Contributor

DavidBadura commented Nov 15, 2019

thanks @lyrixx for this contribution :)

I tried it and found the problem that the logging recrudes. The handler uses the HttpClient, which logs every request (POST http://127.0.0.1:9200/_bulk). I had to exclude the channel http_client, otherwise I hit the memory limit.

monolog:
    handlers:
        es:
            type: service
            id: Symfony\Bridge\Monolog\Handler\ElasticsearchLogstashHandler
            channels: ["!http_client"]

Do you have any idea how to still log the http client requests without landing in a recrusion?
And I think, the problem should also be documented.

@DavidBadura
Copy link
Contributor

DavidBadura commented Nov 16, 2019

I bypassed this problem by not using the global http client:

    Symfony\Bridge\Monolog\Handler\ElasticsearchLogstashHandler:
        arguments:
            $client: null

so I can remove the http_client channel exclude in the mongo handler config :)

@lyrixx
Copy link
Member Author

lyrixx commented Nov 16, 2019

Hello. I did not anticipate this 😂
I'm working on a fix.

@lyrixx
Copy link
Member Author

lyrixx commented Nov 16, 2019

Hmm, I don"t see how Symfony could fix this issue since it's "normal" to have this behavior.
So instead I submitted a PR to the doc repo symfony/symfony-docs#12642
and I fixed my article https://jolicode.com/blog/how-to-visualize-symfony-logs-in-dev-with-elasticsearch-and-kibana

Thanks for the feedback 👍

lyrixx added a commit to symfony/monolog-bundle that referenced this pull request Jan 6, 2020
This PR was merged into the 3.x-dev branch.

Discussion
----------

Use an HttpClient without logger in all handlers

fixes:

* symfony/symfony#34423
* symfony/symfony#32360 (comment)
* symfony/symfony-docs#12642

Commits
-------

faf6943 Use an HttpClient without logger in all handlers
dani-danigm pushed a commit to dani-danigm/monolog-bundle that referenced this pull request Jun 15, 2022
This PR was merged into the 3.x-dev branch.

Discussion
----------

Use an HttpClient without logger in all handlers

fixes:

* symfony/symfony#34423
* symfony/symfony#32360 (comment)
* symfony/symfony-docs#12642

Commits
-------

2abc6f6 Use an HttpClient without logger in all handlers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants