search

mozilla / fxa-auth-server

DEPRECATED - Migrated to https://github.com/mozilla/fxa
Mozilla Public License 2.0
399 stars 121 forks source link

fix(logs): rename `code` to `status` on request.summary log lines #2886

Closed philbooth closed 5 years ago

philbooth commented 5 years ago

Fixes #2885.

Stackdriver doesn't like it when properties change their type across log lines, and there is a clash between the type of the code property for the account.signin.confirm.success and request.summary log lines.

This change renames it to status on request.summary, because that seems like a more appropriate name and the other log message appears to have greater legitimacy to its claim to use the code property name (that is literally the name of the corresponding property in the request payload).

However, I also know that request.summary is going to be parsed by the fraud detection pipeline, so maybe this is a breaking change for them. If that is the case I can change the other log line instead, no probs. @shane-tomlinson, maybe you know for certain whether that is the case (or can point me towards the right person to ask to find out)?

@mozilla/fxa-devs r?

philbooth commented 5 years ago

Hey @ameihm0912, this PR proposes to change a property name in the auth server request.summary log line. I know you guys are going to parse these lines for fraud detection, so I wanted to check we're not going to break anything by making this change.

Specifically, the HTTP status code, which is currently set in the code property, would be set in the status property instead. How does that sit with you guys?

ameihm0912 commented 5 years ago

@philbooth that sounds good, r+. Also we had a look through our existing parsers/analysis plugins and nothing is currently using this field from request.summary.