Live pa connectivity status logs #865
Conversation
- Refactor logging to be cleaner and more easily extendable - Make sure unit tests actually run on mix test
|
Some thoughts on the refactor: ➕ I think the change to having the device-specific functions return keyworded fields, instead of doing the logging and string formatting within each one, is a nice improvement. ➖ I'm less convinced about the change to how these functions are called. Previously the conditional on "device type" was handled as part of the pattern-matching for each function head, which puts the entire "format" of the input message in one place. With this change, we match on just the "device type" part of the message and use that value to dispatch to a device-specific function — which is essentially the same thing, but with an extra layer of indirection. Reading between the lines a bit, it seems like you might have made this change to account for unknown device types, which are handled in a unique way. What if instead we kept the shared function approach to translate a message into fields (using pattern-matching as the conditional, as before), but had this return either Open to others' thoughts, in case I'm the only one who saw it this way. |
lib/monitoring/uptime.ex
Outdated
| Enum.reduce(fields, base, fn {k, v}, acc -> | ||
| acc ++ [" #{Atom.to_string(k)}=", v] | ||
| end) | ||
| |> Logger.info() |
There was a problem hiding this comment.
This approach is good, but the code could be tuned up a bit:
- Logging via lists doesn't behave well if e.g. a number slips through, so it would be safer to send everything through interpolation.
date_timecould be prepended as a tuple to the field list, instead of being formatted separately.reducecould be replaced with a slightly more expressivemapandjoinAtom.to_stringis unnecessary if it's being interpolated
Summary of changes
Asana Ticket: Process ARINC JSON for LivePA connectivity into Splunk
Adds handling for Live PA Connectivity statuses. Also took the opportunity to refactor and simplify some things to improve readability and extensibility.
Note: some field names were changed to make things a little more standardized. This will affect a couple Splunk alerts and so we should give Kevin a heads up before this is deployed.
Reviewer Checklist
signs.jsonwas changed, there is a follow-up PR tosigns_uito update its dependency on this project