Support or improve bracket logging for JSON arrays

[sir4ur0n]

Currently (Katip 0.8.7.4) the function renderPair (used by toBuilders and getKeys, hence used for bracket logging) ignores JSON arrays entirely: https://github.com/Soostone/katip/blob/master/katip/src/Katip/Scribes/Handle.hs#L60

_ -> mempty -- Can't think of a sensible way to handle arrays

While I understand this is not trivial (after all, the symbols [] are already reserved as delimiters), I think Katip should:

• either log the key, and put a generic value, e.g., [myPayloadKey:<array_fields_are_not_supported_by_Katip>]
• or log the full array by picking a different delimiter, e.g., use |, as in [myPayloadKey:|123, "abcd", true|]

I also think that, if full support for arrays is not added, all functions impacted by this limitation should have some documentation to warn users.

For the record, we have spent a significant amount of time in my team trying to figure out what was wrong with our logs, because those were silently ignored 😅

What do you think about those suggestions?

[MichaelXavier]

I think the second option is probably better. I think people will get even more annoyed if we log a placeholder message in each log. We could possibly omit the tokens surrounding the array, e.g. [myPayloadKey: 123, "abcd"]. Or we could do what we do for objects but treat each 0-indexed index as if it's a property of an object myPayloadKey.0, myPayloadKey.1, etc. What do you think?

[sir4ur0n]

Or we could do what we do for objects but treat each 0-indexed index as if it's a property of an object myPayloadKey.0, myPayloadKey.1, etc.

While this becomes less practical when the payload gets big, I like the consistency with object values. This follows the principle of least astonishment so I think this is a great idea 👍

[MichaelXavier]

Sounds good. Please feel free to submit an MR and I'll take a look. Thanks!

[sir4ur0n]

@MichaelXavier Done in #150

[sir4ur0n]

BTW I am a Nix/NixOS user and there was no Nix shell file in the repo.

I quickly put together this file to be able to work on Katip:

with import <nixpkgs> { };
# We add our packages to the haskell package set
(haskellPackages.extend (haskell.lib.compose.packageSourceOverrides {
  katip = ./katip;
  katip-datadog = ./katip-datadog;
  katip-elasticsearch = ./katip-elasticsearch;
  katip-logzio = ./katip-logzio;
}))
# We call on this set shellFor to drop us into a shell containing the dependencies of frontend and backend:
.shellFor {
  packages = p: [
    p.katip
    p.katip-datadog
    p.katip-elasticsearch
    p.katip-logzio
  ];
  withHoogle = true;
  buildInputs = [ pkgs.cabal-install ];
}

It's not pure because of <nixpkgs> but this is a decent first approximation.

Would you be interested in me opening another PR to add this Nix shell file?

[MichaelXavier]

I think for now we'll just stick with stack since it's pretty ubiquitous. We use nix ourselves but I don't want to open the can of worms of how we want to set it up with open source projects quite yet.