[3.x] feat: OhDear integration

[kbond]

@Chris53897, as it looks like you'd use this, could you give a quick review?

[Chris53897]

I will do as sonn if i have some free time. But we do not use LiipMonitorBundle in Production yet.

[Chris53897]

I just noticed that laminas/diagnostics is now compatible with PHP 8.3. This makes testing for me easier, as we already moved all our projects and bundles to PHP 8.3 as minimum. Will try to do it in the next days.

[Chris53897]

Thanks. It looks good. Some ideas for improvements/considerations.

If i activate the check for system_memory_usage: true # warn @ 70%, fail @ 90% it results in crashed, because not supported on mac silicon accoring to the message.

The stackTrace is in the MetaData stack_trace. Should this be excluded for security reasons?

The documentation is not clear if the correct status is error or failed. See notificationMessage https://ohdear.app/docs/features/application-health-monitoring#health-check-results-format I did contact support of OhDear for this.

Maybe it is useful to make it easier in env=dev to show the health results as json. By direct call via Browser Somethink like this, but needs a check for dev env. And on the downside, it logs the sectet in logs, and can leaked. So i am not sure if there is a better solution if ($this->secret !== $request->headers->get('oh-dear-health-check-secret') && $this->secret !== $request->get('oh-dear-health-check-secret')) {

I think a hint to add the route to the firewall is helpful.

checkResults is limited to 50 items. I am not sure how OhDear handle this, if there are more. https://ohdear.app/docs/features/application-health-monitoring#health-check-results-format I did contact support of OhDear for this. Update: If there are more than 50, no import will be done. Maybe sort the items by status? Failing and crashing first. Discard by 49 and optional add a warning that there are more than 50 checks?

[kbond]

Great, thanks for testing this!

If i activate the check for system_memory_usage: true # warn @ 70%, fail @ 90% it results in crashed, because not supported on mac silicon accoring to the message.

Do you think we should do anything here? I'm guessing it will work in production?

The stackTrace is in the MetaData stack_trace. Should this be excluded for security reasons?

Good call - global config option perhaps? I guess in certain scenario's you may want the stack trace. Maybe an option when running the checks?

I think a hint to add the route to the firewall is helpful.

Can you describe this a bit more? The route needs to be public (or is that what you mean?).

Maybe sort the items by status? Failing and crashing first. Discard by 49 and optional add a warning that there are more than 50 checks?

Good idea, that seems like the best option.

[kbond]

Maybe it is useful to make it easier in env=dev to show the health results as json.

What about wrapping the request header check into a protected method that you can override to add this type of logic?

[Chris53897]

@kbond Sorry, for late response. You have good Points.

Maybe we can merge this PR as it is, and after that tackle one task after the other? This will make it easier to contribute for others.

Our symfony workers in live actual get stucked after some load, and we need to check it manual at the moment ;( I would love to have a roundtrip of checking # workers running => LiipMonitoringBundle => OhDear => ReRun Workers on live, if stuck. Or a cronjob that checks # workers running and restart them if stuck. But the other checks mentionted here, are useful as well.

https://github.com/zenstruck/messenger-monitor-bundle/issues/11

[kbond]

Maybe we can merge this PR as it is, and after that tackle one task after the other?

Good idea, merging.