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

ensures PSR log level can be int or string #684

Merged
merged 1 commit into from
Sep 26, 2017

Conversation

bshaffer
Copy link
Contributor

casts $level to an integer to ensure it can be handled by gRPC in PsrLogger. Otherwise, passing in a string (such as "emergency") raises the following error:

PHP Notice:  Undefined index: warning in vendor/google/cloud-logging/Connection/Grpc.php on line 315

Notice: Undefined index: warning in vendor/google/cloud-logging/Connection/Grpc.php on line 315

                   
  [Exception]      
  Expect integer.  
                   

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 23, 2017
@bshaffer bshaffer requested a review from dwsupplee September 23, 2017 01:28
@jdpedrie
Copy link
Contributor

jdpedrie commented Sep 24, 2017

Hey @bshaffer, looks like this change will require system test updates as well:

➜ vendor/bin/phpunit -c phpunit-system.xml.dist --group=logging
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.

.............................E.E.E.E.E.E.E.E

Time: 1.26 minutes, Memory: 18.00MB

There were 8 errors:

1) Google\Cloud\Tests\System\Logging\WriteAndListEntryTest::testWritesEmergencyLogWithPsrLogger with data set #1 (Google\Cloud\Logging\LoggingClient Object (...))
Undefined offset: 800

./google-cloud-php/src/Logging/Connection/Grpc.php:315
./google-cloud-php/src/Logging/Connection/Grpc.php:125
./google-cloud-php/src/Logging/Logger.php:389
./google-cloud-php/src/Logging/Logger.php:358
./google-cloud-php/src/Logging/PsrLogger.php:491
./google-cloud-php/src/Logging/PsrLogger.php:390
./google-cloud-php/src/Logging/PsrLogger.php:153
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:237
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:165

2) Google\Cloud\Tests\System\Logging\WriteAndListEntryTest::testWritesAlertLogWithPsrLogger with data set #1 (Google\Cloud\Logging\LoggingClient Object (...))
Undefined offset: 700

./google-cloud-php/src/Logging/Connection/Grpc.php:315
./google-cloud-php/src/Logging/Connection/Grpc.php:125
./google-cloud-php/src/Logging/Logger.php:389
./google-cloud-php/src/Logging/Logger.php:358
./google-cloud-php/src/Logging/PsrLogger.php:491
./google-cloud-php/src/Logging/PsrLogger.php:390
./google-cloud-php/src/Logging/PsrLogger.php:170
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:237
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:173

3) Google\Cloud\Tests\System\Logging\WriteAndListEntryTest::testWritesCriticalLogWithPsrLogger with data set #1 (Google\Cloud\Logging\LoggingClient Object (...))
Undefined offset: 600

./google-cloud-php/src/Logging/Connection/Grpc.php:315
./google-cloud-php/src/Logging/Connection/Grpc.php:125
./google-cloud-php/src/Logging/Logger.php:389
./google-cloud-php/src/Logging/Logger.php:358
./google-cloud-php/src/Logging/PsrLogger.php:491
./google-cloud-php/src/Logging/PsrLogger.php:390
./google-cloud-php/src/Logging/PsrLogger.php:187
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:237
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:181

4) Google\Cloud\Tests\System\Logging\WriteAndListEntryTest::testWritesErrorLogWithPsrLogger with data set #1 (Google\Cloud\Logging\LoggingClient Object (...))
Undefined offset: 500

./google-cloud-php/src/Logging/Connection/Grpc.php:315
./google-cloud-php/src/Logging/Connection/Grpc.php:125
./google-cloud-php/src/Logging/Logger.php:389
./google-cloud-php/src/Logging/Logger.php:358
./google-cloud-php/src/Logging/PsrLogger.php:491
./google-cloud-php/src/Logging/PsrLogger.php:390
./google-cloud-php/src/Logging/PsrLogger.php:204
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:237
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:189

5) Google\Cloud\Tests\System\Logging\WriteAndListEntryTest::testWritesWarningLogWithPsrLogger with data set #1 (Google\Cloud\Logging\LoggingClient Object (...))
Undefined offset: 400

./google-cloud-php/src/Logging/Connection/Grpc.php:315
./google-cloud-php/src/Logging/Connection/Grpc.php:125
./google-cloud-php/src/Logging/Logger.php:389
./google-cloud-php/src/Logging/Logger.php:358
./google-cloud-php/src/Logging/PsrLogger.php:491
./google-cloud-php/src/Logging/PsrLogger.php:390
./google-cloud-php/src/Logging/PsrLogger.php:221
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:237
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:197

6) Google\Cloud\Tests\System\Logging\WriteAndListEntryTest::testWritesNoticeLogWithPsrLogger with data set #1 (Google\Cloud\Logging\LoggingClient Object (...))
Undefined offset: 300

./google-cloud-php/src/Logging/Connection/Grpc.php:315
./google-cloud-php/src/Logging/Connection/Grpc.php:125
./google-cloud-php/src/Logging/Logger.php:389
./google-cloud-php/src/Logging/Logger.php:358
./google-cloud-php/src/Logging/PsrLogger.php:491
./google-cloud-php/src/Logging/PsrLogger.php:390
./google-cloud-php/src/Logging/PsrLogger.php:238
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:237
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:205

7) Google\Cloud\Tests\System\Logging\WriteAndListEntryTest::testWritesInfoLogWithPsrLogger with data set #1 (Google\Cloud\Logging\LoggingClient Object (...))
Undefined offset: 200

./google-cloud-php/src/Logging/Connection/Grpc.php:315
./google-cloud-php/src/Logging/Connection/Grpc.php:125
./google-cloud-php/src/Logging/Logger.php:389
./google-cloud-php/src/Logging/Logger.php:358
./google-cloud-php/src/Logging/PsrLogger.php:491
./google-cloud-php/src/Logging/PsrLogger.php:390
./google-cloud-php/src/Logging/PsrLogger.php:255
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:237
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:213

8) Google\Cloud\Tests\System\Logging\WriteAndListEntryTest::testWritesInfoDebugWithPsrLogger with data set #1 (Google\Cloud\Logging\LoggingClient Object (...))
Undefined offset: 100

./google-cloud-php/src/Logging/Connection/Grpc.php:315
./google-cloud-php/src/Logging/Connection/Grpc.php:125
./google-cloud-php/src/Logging/Logger.php:389
./google-cloud-php/src/Logging/Logger.php:358
./google-cloud-php/src/Logging/PsrLogger.php:491
./google-cloud-php/src/Logging/PsrLogger.php:390
./google-cloud-php/src/Logging/PsrLogger.php:272
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:237
./google-cloud-php/tests/system/Logging/WriteAndListEntryTest.php:221

@bshaffer
Copy link
Contributor Author

Good catch, @jdpedrie. This is now fixed!

@dwsupplee
Copy link
Contributor

Great find!

There is already a bit of logic touching on this - WDYT about modifying

https://github.com/GoogleCloudPlatform/google-cloud-php/blob/fa7977f7839b46a594cbef37a7d572127d10633b/src/Logging/Connection/Grpc.php#L314-L316

to

if (isset($entry['severity']) && is_string($entry['severity'])) {
    $entry['severity'] = array_flip(Logger::getLogLevelMap())[strtoupper($entry['severity'])];
}

and keeping the validate function from modifying the level?

@bshaffer bshaffer force-pushed the fix-psr-logger-level branch from 78eaceb to 6055d22 Compare September 26, 2017 22:14
@bshaffer
Copy link
Contributor Author

@dwsupplee good call, that works way better.

@dwsupplee dwsupplee merged commit 65ab8d9 into googleapis:master Sep 26, 2017
@bshaffer bshaffer deleted the fix-psr-logger-level branch September 26, 2017 23:48
@jdpedrie jdpedrie mentioned this pull request Oct 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants