Log metadata about Attempts API events (LG-7562)#6963
Conversation
changelog: Internal, Logging, Log metadata about Attempts API
|
|
||
| def track_event(event_type, metadata = {}) | ||
| return unless enabled? | ||
| return if !enabled? && !IdentityConfig.store.irs_attempt_api_payload_size_logging_enabled |
There was a problem hiding this comment.
This is perhaps the only case where unless ... || ... would read cleanly to me since we're negating both.
That's generally seen as a cardinal sin, though, so I'm fine with this as-is.
There was a problem hiding this comment.
It's my personal vendetta against unless. Another idea was an explicit negation/or. What do you think of this?
| return if !enabled? && !IdentityConfig.store.irs_attempt_api_payload_size_logging_enabled | |
| return if !(enabled? || IdentityConfig.store.irs_attempt_api_payload_size_logging_enabled) |
There was a problem hiding this comment.
going to leave it as is for the time being but open to changing later if this confuses folks!
There was a problem hiding this comment.
My brain has a hard time with !( ... || ... ) / !( ... && ... ) doing mental De Morgan's to decipher the logic, so I'm happier with it as is, and would have also been fine with unless (I don't share the same vendetta 😄 ).
3 participants
Also adds a feature flag in case we discover this has some sort of performance hit to generate and JSONify and throw away all these strings