search

magento / meta-for-magento2

33 stars 19 forks source link

No ability to prevent logs sending from Meta Extension to meta #33

Open ihor-sviziev opened 9 months ago

ihor-sviziev commented 9 months ago

I noticed that this extension sends a part of the logs to Meta. There is supposed to be an ability to enable/disable this functionality.

Related functionality:

Preconditions (*)

1. 2.

Steps to reproduce (*)

1. 2.

Expected result (*)

  1. [Screenshots, logs or description]

Actual result (*)

  1. [Screenshots, logs or description]
zlik commented 9 months ago

@nrostrow-meta

nrostrow-meta commented 9 months ago

@ihor-sviziev There currently is the ability to prevent the exception logs from being sent to Meta. Though it might not be adequate so please let me know if so. The logExceptionImmediatelyToMeta function just adds 'log_type' to the context and then calls the logException function. The logException function has a check for if sending logs to Meta is enabled and if it's not enabled then the log is just made locally and is not added to the queue to be persisted to Meta.

https://github.com/magento/meta-for-magento2/blob/3ba54355fa5b1efd867ee0b59787e917efd9ab66/app/code/Meta/BusinessExtension/Helper/FBEHelper.php#L306-L311

Enabling/disabling persisting exception logs to Meta can be controlled by updating the meta_exception_logging_enabled config value.

https://github.com/magento/meta-for-magento2/blob/3ba54355fa5b1efd867ee0b59787e917efd9ab66/app/code/Meta/BusinessExtension/etc/config.xml#L11

Currently this config value isn't shown in the UI though and can only be modified in code. By default the value is set to true to enable persisting exception logs to Meta. The reason for this is that the exception logging is pretty critical for us to be able to identify and diagnose issues with the extension so we were afraid that if we showed it in the UI that sellers would turn it off by default.

https://github.com/magento/meta-for-magento2/blob/3ba54355fa5b1efd867ee0b59787e917efd9ab66/app/code/Meta/BusinessExtension/etc/adminhtml/system.xml#L48-L53

Is this acceptable or does disabling sending exception logs to Meta need to be prevalent in the UI? Thanks!

sol-loup commented 1 month ago

@ihor-sviziev -- if this is acceptable, do you mind resolving this issue?