No logging for send_request() calls in VIP environment

[rahulsprajapati]

Describe the bug

Whenever we get any non 2xx response from Brightcove API ( send_request() ) we doesn't have the information on logs to track what was the issue on VIP environment.

ref:

send_request():

https://github.com/10up/brightcove-video-connect/blob/f7a67aca4d20c9e7ac53b1a6cf26b5f9b9c37db1/includes/api/class-bc-api.php#L250-L292

BC_Logging::log()

https://github.com/10up/brightcove-video-connect/blob/f7a67aca4d20c9e7ac53b1a6cf26b5f9b9c37db1/includes/class-bc-logging.php#L58-L62

Is there any specific reason we are not writting logs for VIP environment? If not we can use newrelic_notice_error for VIP environment.

ref: https://wpvip.com/documentation/manually-logging-errors-in-new-relic/

Expected behavior Use newrelic_notice_error for VIP environment and add logs in newrelic.

Additional context

We are using this plugin on multiple sites and we had one issue where we were getting errors from API for few min and it got fix automatically we checked with brightcove team but they didn't had any service intruption which could cause any issue and since the issue got fixed automatically it couldn't be an issue of credentials / token authentication issue. And there was no error log added anywhere so we couldn't find the root cause. Just checking here if we can add logs on VIP environment it would help in future issues like this.

[oscarssanchez]

Hi @rahulsprajapati , Can you try with this branch please see if it solves the issues for you? https://github.com/10up/brightcove-video-connect/tree/feature/VIP-error-logging Thanks!

[rahulsprajapati]

Sure, @oscarssanchez. I'll check this and get back to you soon. Thanks!

[rahulsprajapati]

Hello @oscarssanchez

I've looked at the changes from https://github.com/10up/brightcove-video-connect/commit/63e764ca6413930ce78e92b81a64ad907b6436f4, looks good.

Just have one suggesion: Instead of:

if( $is_vip ) {
    newrelic_notice_error( $message );
} else {
    error_log( $message );
}

can we add this?

if( $is_vip && function_exists( 'newrelic_notice_error' ) ) {
    newrelic_notice_error( $message );
} else {
    error_log( $message );
}

On VIP newrelic is just running on half of the container which can cause us losing this logs. VIP does keep php logs from error_logs and can provide that on request. ref: https://wpvip.com/documentation/vip-go/new-relic-on-vip-go/#which-parts-of-the-vip-infrastructure-is-new-relic-installed-on

We could use extension_loaded( 'newrelic' ) also instead function_exists( 'newrelic_notice_error' ) ref: https://wpvip.com/documentation/manually-logging-errors-in-new-relic/

[oscarssanchez]

This sounds great @rahulsprajapati , thanks, i have adjusted the code and expecting to push it in our next release.

[rahulsprajapati]

Thank you! @oscarssanchez

[oscarssanchez]

Closing per #183