Failed again in: https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?buildbucketId=8924242727641122208, seems like a flaky issue.

vapier@ or benchan@ Could you please take a look and see if this something you can fix?

This same error ("Report should not have signature") caused:
- Last night's PFQ build to fail: https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?buildbucketId=8924180808655505632
- Most recent peach_pit-tot-chrome-pfq-informational: https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?buildbucketId=8924178119767498400

i stopped paying attention to flaky failures as they pretty much were all issue 740422.  this one might be a "real" flake which is due to the test framework.

vapier@, issue 740422 seems to be related to a permission denied error when calling getcwd. This one however seems to be specific to crash reporting. Are they really related?

I'm not sure which signature that is that shouldn't be in the report, because this line [1] in the test code produces the following log in the test logs:

01/14 02:13:47.690 DEBUG|   user_crash_test:0311| Crasher process result: {'crashed': True, 'returncode': 139, 'output': 'pid=20731\nCrashing as requested.\n', 'crash_reporter_caught': True}

As you can see, no 'sig' key in the result. Yet, this [2] later complains about its existence:

TestFail: Report should not have signature

[1]: https://cs.corp.google.com/chromeos_public/src/third_party/autotest/files/client/cros/crash/user_crash_test.py?type=cs&q=%22Report+should+not+have+signature%22+p:chromeos_&g=0&l=311

[2]: https://cs.corp.google.com/chromeos_public/src/third_party/autotest/files/client/cros/crash/user_crash_test.py?type=cs&q=%22Report+should+not+have+signature%22+p:chromeos_&g=0&l=368

We have a similar failure in the CQ

TestFail: Report signature mismatch: -66, vs crash_reporter-user-collection

https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?buildbucketId=8924077289994238816

I am wondering if it's the same flake.  Should I file a separate bug?  Or maybe whoever looks at this can decide.

BTW we need an owner for this.  Suggestions are welcomed.  Volunteers are even more welcomed!

Oh wait.  

kernel | dawid.kurek-AT-displaylink.com | 1411588 | drm/evdi: Log process executable name along with PID

this might change the crash signature.

https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1411588

Seems very specific though.  Not sure this can happen in the autotest.

LOL, this has some terrible test code. It's basically grepping for 'sig:' in our logs. So we're triggering on stuff like this:

Jan 14 05:51:03 shill[1579]: [INFO:wifi_service.cc(786)] Rep ep updated for 65. [SSID=GoogleGuest-Legacy], bssid: 94:b4:0f:ff:d0:e0, sig: -70, sec: none, freq: 2412


But...that's not normally in syslog. Oh, wait, we're starting to use journalctl, which merges in shill logs? I bet that caused this then.

Thank you Brian!
Let's disable this test until it's fixed.

(BTW, I came here because I saw a paladin failing on this.)

@10: Sure, assigning to you disabling :) If I'm reading correctly, then that's the only test directly looking for 'sig:' in this way. But anything using the 'crash_test' / 'user_crash_test' modules could be subject to similar errors, because of this in cros_logging:

    if os.path.exists("/var/log/journal"):
        return JournalLogReader()
    else:
        return LogReader()

which suddenly means there are a lot more logs going into this log-parsing code. The surest fix would be to back out some of b/120686750 I expect. But that's really not the main problem, so do what you prefer.

I might also take a stab at tightening up the crash-log parsing in the meantime, to fix this properly.

I have two CLs.  This one disables the test:
https://crrev.com/c/1419220

This one fixes it.
https://crrev.com/c/1418986

We should probably just fix it because it's simple.

shill normally sends it's logs to syslog. We have syslog configured to put log messages from shill and certain other sources into  /var/log/net.log. Note that messages of ERR or more important also go into /var/log/messages. /var/log/messages is just the default location for log messages from sources that don't have their own file. journald shouldn't change any of this since it just forwards messages to syslog. As mentioned in comment#11, this seems to be due to existing code added by the lakitu folks to read journal logs when they exist. I had no idea that it was there, but it'll make some of the work to moving to journald easier :)

It was recommended that I back out the journald change and add it back in after the 73 branch next week in order to give it more bake time. I think it's a good idea, so I'm going to go ahead and do it. The underlying tests should still be fixed though.

The underlying test is so broken and the fix is (I think) so easy that I suggest we go that way instead.

> journald shouldn't change any of this since it just forwards messages to syslog.

The effective change is because shill's INFO logs now show up in journalctl (and, as noted, the existing test logic for lakitu is picking this up instead of /var/log/messages).

re-comment #5: i'm not saying this is issue 740422.  because of that issue, i've ignored flakes in any of these tests because they've pretty much all been bugs in the autotest framework.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/f486553a988458f5ccae9089b84a0c76988a345d

commit f486553a988458f5ccae9089b84a0c76988a345d
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Fri Jan 18 02:06:03 2019

crash_test: make RE searches more precise.

The immediate bug is caused by searching for "sig: (\S+)", which
incorrectly matches log lines from shill.  Other patterns were
at risk too.

BUG= chromium:921679 
TEST=ran the test

Change-Id: I30c7b35e02fb1b9d6e66dc4b010197a6f495f7b9
Reviewed-on: https://chromium-review.googlesource.com/c/1418986
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Commit-Queue: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/f486553a988458f5ccae9089b84a0c76988a345d/client/cros/crash/crash_test.py

thanks guys for tracking it down