Pierre-Lannoy / wp-decalog

Capture and log events, metrics and traces on your site. Make WordPress observable - finally!
https://perfops.one/
GNU General Public License v3.0
64 stars 8 forks source link

[Datadogformatter] Reduced set of status level used for event submission #53

Closed JanThiel closed 1 year ago

JanThiel commented 1 year ago

Hey @Pierre-Lannoy,

more of a question but something we came along trying to do some housekeeping in datadog.

Within https://github.com/Pierre-Lannoy/wp-decalog/blob/33f890c8e9cd40b8cc50e18e412734a877f404c4/includes/formatters/class-datadogformatter.php#L41-L48 you limit the used status levels to info, warning and error although DD does support all of them actually. We stumbled upon this wondering why all NOTICES where categorized as info in DD and expected some wrong at our pipeline config. Our particular usecase: We store logs depending on their level. Like info can go away after some days whereas warning and error will be stored for 30 days. Now we wanted to split info and notice to store notice longer and drop info even quicker.

This helps us optimize mostly the cost structure as well as little bit reducing log bloat in DD ;-).

Just want to get your resoning behind this. As I belive this is intentional as always :-) If you agree, I would at least move Decalog / PHP notice to DD notice. If not even setting all of the to their according DD log levels. But that seems a bit "radical" to me at the moment with no actual need.

Have a great day and all the best as usual :-)

Pierre-Lannoy commented 1 year ago

Hello @JanThiel !

Hope you are well :)

As I'm working on DecaLog 3.8.0 (to be ready for WP 6.3), I would like to answer your question. Why the f**ck did I have mapped levels and Datadog status? 🤣 That's a good question, and the answer is: because when I did it the first time, Datadog was not supporting other status than the 3 I implemented via json post (so in "deferred forwarding", as it is called in DecaLog). I haven't been a Datadog user for a while, but your question made me wonder... What if they had opened up the json specifications to missing statuses? So I made a few tests and… Wowowow: it's now a "open" field: you can do what you want - almost.

So, of course, DecaLog will implements all statuses. I have to make some tests again but, so far so good:

DD

As all status are now catched, I plan to remove the colored square, the level in plain text and the error code in the message log. What do you think about it?

In any case, I wanted to thank you a thousand times over for pointing this out to me. If you hadn't, I'd never have come back to see if it was possible.

Thanks ❤️

JanThiel commented 1 year ago

@Pierre-Lannoy As always - it is a pleasure feeding you with input and critical questions :-) Thank you very much for the reasoning and fixing it ;-)

Pierre-Lannoy commented 1 year ago

Thanks for your feedback :) This change will be released in 3.8.0 (in 4 days if all go well).