Error Reporting sample is not working. about php-docs-samples

[tmatsuo]

@steren Thanks for the report, I have few questions.

What happens with the first code? Is the log lost? Is it stored, but in the wrong format?

appropriate format

Where is it defined? The sample you showed seems weird, because it always says PHP Notice.
Why is the string "PHP Notice: " required?

If you have a plan for fixing this, can you share the details of your plan? ETA, how will you fix, etc?

Thanks!

[steren]

This sample is used for Error Reporting documentation, not for Logging.

In the first code, the error is just stored in Logs, but not processed by Error Reporting because it is not in the expected format.

While waiting for Error Reporting to be more flexible in its error parsing (no ETA yet), we should adapt our samples to what Error Reporting is able to parse today. "PHP Notice" is required to fit the current parser.

The exception format is not documented, the idea is that Error Reporting is supposed to process default exception format.

Keeping a broken sample doesn't make sense and is very misleading. We will update it again later once the parser is updated.

[tmatsuo]

@steren
Then how about turn down the sample? Isn't it also confusing we keep changing our samples?

[steren]

GCE support is in Beta, it is OK to change our sample over time.

The fix I suggest has unblocked a customer who uses PHP.

I do not understand this push back and why we would not document how to use Error Reporting with PHP, knowing we have a workaround to the parser limitation

[tmatsuo]

The reason I'm hesitant is because the string "PHP Notice:" looks really weird.
@bshaffer WDYT?

[steren]

I agree it looks weird, we can add a comment explaining that it is due to a limitation of the Error Reporting service and will change in the future.

[bshaffer]

Yeah, this is a pretty unfortunate solution. What if Exception is a fatal error? We are logging it as a Notice, which is considered a lower level than Warning.

If anything, these errors should be logged as Emergency, as an exception handler indicates a 500 error. @steren Is there another option here, or is Notice the only way?

[steren]

Here are the current prefixes that will be identified by the parser: PHP (Notice|Parse error|Fatal error|Warning):. We can pick any of this list at the moment, maybe Warning is better suited?

But note that the log entry severity is decorrelated from this. The severity can be set with the severity attribute.

So I suggest to update to:

$msg = array(
        'message' => sprintf("PHP Warning: %s", (string)$e),
        'serviceContext' => array('service' => 'myapp'),
        'severity'=>'ERROR'
        // ... add more metadata
    );

[steren]

If everything goes well, the parser fix should be deployed by the end of the year or early next year. But in the meantime, I still think we should fix our broken sample.

[tmatsuo]

@steren

I don't disagree that we should fix broken samples, but at the same time, we have finite resource. Considering the timing, the fix will be very short lived. We don't want to spend our time to write soon-to-be-thrown-away and looking-very-weird samples. So I suggested that we just turn down the sample and revive it when it just works.

However, what's the plan for the fix? Can we just pass the $e-getMessage() then will it work? Or will we still need to add some prefix?

That's why I wanted to know the future plans, specs etc.

The new sample you suggested look much nicer, so I'm not worried about the weirdness any more. We will fix it if time allows.

[steren]

After the fix, you will need to pass 'message' => (string)$e,.

$e->getMessage() does not print a stack trace (and a stack trace is required for error grouping).

[tmatsuo]

I think users should add severity if it matters, right?

[steren]

Yes, adding a severity in the payload probably is better for users. But is not a hard requirement for errors to be processed.

So after the parser fix, the sample should be:

$msg = array(
        'message' => (string)$e,
        'serviceContext' => array('service' => 'myapp'),
        'severity'=>'ERROR'
        // ... add more metadata
    );

[chooh]

(ಠ_ಠ)

 self::fluentLogger()->post('errors', [
   'serviceContext' => ['service' => 'balance-checker'],
-  'message' => $e->getMessage() . PHP_EOL . "Stack trace:" . PHP_EOL . $e->getTraceAsString(),
+  'message' => str_replace('warning', 'Warning', $e->getMessage()) . PHP_EOL . "Stack trace:" . PHP_EOL . $e->getTraceAsString(),
 ]);

After this patch it finally worked.

[steren]

Thanks. The second sample I provided in my original report should work too since the parser still expects this prefix: PHP (Notice|Parse error|Fatal error|Warning)