search

IntersectMBO / cardano-node

The core component that is used to participate in a Cardano decentralised blockchain.
https://cardano.org
Apache License 2.0
3.07k stars 720 forks source link

[BUG] - Integers display in scientific notation in SimpleView #2272

Open rphair opened 3 years ago

rphair commented 3 years ago

Internal/External External Area Monitoring Summary When using SimpleView, with node 1.24.2 the block producer node began displaying slot numbers for each leadership check in scientific notation. Since these values are either integers or discrete values by nature, showing them as floating point numbers is a senseless complication for programs that process logfiles as well as the human reader.

Steps to reproduce 1) Put a block producer node in SimpleView via its config.json settings. 2) Substituting your logfile for node.log if necessary, run (assuming log file begins with 1.24.2 log output):

$ grep 'fromList .("credentials",String "Cardano")' node.log | head -1

3) See the integer slot number display in scientific notation: [core-nyc:cardano.node.Forge:Info:53] [2020-12-09 18:45:14.17 UTC] fromList [("credentials",String "Cardano"),("val",Object (fromList [("kind",String "TraceNodeNotLeader"),("slot",Number 1.5973223e7)]))]

Expected behaviour Display whole numbers as whole numbers, for compatibility with explorers, concisely written scripts, human readers using grep for quick analysis (without a painstaking conversion to scientific notation), etc.

Human readers trying to make sense of scrolling log files, or write scripts to look up block successes & failures, will wonder why the "New tip" message is displayed as an integer and the leader check is using scientific notation for the same data... which creates difficulties searching back & forth between these two contexts:

$ grep 'Chain extended, new tip:' node.log | head -1

[core-nyc:cardano.node.ChainDB:Notice:43] [2020-12-09 18:45:14.19 UTC] Chain extended, new tip: 47e734df76f1e1c5af3c08cdce761bb7658e624d840f34c8c979493b1ed74c3b at slot 15973213

System info

Additional context This is not the only context where a value is displayed as floating point when its presentation as a whole number or fixed precision decimal would make much more sense. For instance chainDensity, a number always slightly less than 0.05, also is displayed only in scientific notation.

I would therefore ask the developers that are dumping all such diagnostic arrays to please consider the human reader and monitoring script writer in presenting this analytical data in whatever form is most easily read & post-processed.

rphair commented 3 years ago

FYI, some attention to this on the Cardano Forum (so far just from me): https://forum.cardano.org/t/slot-battles-what-do-they-look-like-what-can-be-done-about-them/41316/8

Jimbo4350 commented 2 years ago

Closing this. If this is still relevant please re-open.

rphair commented 5 months ago

@Jimbo4350 who knows what would have made you close this, since the original report was observable by anyone who cared for a long time afterward. But FYI someone finally noticed the problem & fixed it, as can be seen in a current text log file message from node version 8.9.3:

[core:cardano.node.LeadershipCheck:Info:304] [2024-05-29 16:40:51.00 UTC] {"chainDensity":4.7917083e-2,"credentials":"Cardano","delegMapSize":1344990,"kind":"TraceStartLeadershipCheck","slot":125434560,"utxoSize":11175231}

i.e. a slot number without the crippling scientific notation (after waiting over 2 years for this to appear as an integer):

"slot":125434560,
rphair commented 2 months ago

@Jimbo4350 it's not only strange that this was closed with no explanation but also that I can't reopen it. If this problem is something the node maintainers don't want to fix, they need to say here why they're not fixing it.

Here's a line from the 9.1.1 node log in SimpleView, highlighting the integers that are appearing in scientific notation:

[core:cardano.node.Forge:Info:404] [2024-09-07 14:29:51.05 UTC] fromList [("credentials",String "Cardano"),("val",Object (fromList [("block",String "729b2ce73ed61bbd75d145d43cdb6e0d1de75b34bdb4110df35389b991c51765"),("blockNo",Number 1.0805114e7),("blockPrev",String "8c6ca9d57ed67f16b87592ad68249755e7f52f7ca5afd70fb1e54ee99b5fccd0"),("kind",String "TraceForgedBlock"),("slot",Number 1.341531e8)]))]

This is the crucial part: the slot number (an integer) doesn't appear in enough precision to tell what the original integer was. How is this logging therefore to be of any use for debugging?

I'd been waiting for this to be fixed in the 9.x overhaul but now assume by default the developers still think the logs don't need to be readable either by humans or scripts. Rather than wait for 10.x I believe this should be fixable any time with a small change to the data types in the routine that is dumping this data structure into the logfile.

Until we get some acknowledgement I would have to keep tagging people that seem responsive about these things, at least until this is reopened & given a proper triage & response please - cc 🙏 @disassembler @erikd @karknu

carbolymer commented 1 month ago

Related:

carbolymer commented 1 month ago

@rphair Is that still the case for integers like blockNo or slotNo? From what I saw, there should be normal integers, without scientific notation. However we still have chain density as scientific notation.

This can be fixed by using custom Value -> ByteString encoder for JSON:

import qualified Data.Aeson as Aeson
import qualified Data.Aeson.Encoding as Aeson
import qualified Data.Aeson.KeyMap as Aeson
import qualified Data.Aeson.Key as Aeson
import qualified Data.Scientific as S
import qualified Data.ByteString.Builder.Scientific as S
import qualified Data.ByteString.Builder as BS
import qualified Data.ByteString.Lazy as BSL

encode' :: ToJSON a => a -> BSL.ByteString
encode' = Aeson.encodingToLazyByteString . Aeson.unsafeToEncoding . encodeToBuilder . toJSON

encodeToBuilder :: Value -> BS.Builder
encodeToBuilder = \case
  (Number n) -> scientific n
  (Array v) -> array v
  (Object m) -> object m
  nonNumber -> toAesonBuilder nonNumber
  where
    -- use native aeson encoding for non-numbers
    toAesonBuilder :: ToJSON v => v -> BS.Builder
    toAesonBuilder = Aeson.fromEncoding . Aeson.toEncoding

    array :: V.Vector Value -> BS.Builder
    array v
      | V.null v  = Aeson.fromEncoding $ Aeson.toEncoding v
      | otherwise = BS.char8 '[' <>
                    encodeToBuilder (V.unsafeHead v) <>
                    V.foldr withComma (BS.char8 ']') (V.unsafeTail v)
      where
        withComma a z = BS.char8 ',' <> encodeToBuilder a <> z

    object :: Aeson.KeyMap Value -> BS.Builder
    object m = case Aeson.toList m of
        (x:xs) -> BS.char8 '{' <> one x <> foldr withComma (BS.char8 '}') xs
        []     -> toAesonBuilder (mempty :: Aeson.KeyMap Value)
      where
        withComma a z = BS.char8 ',' <> one a <> z
        one (k,v)     = key k <> BS.char8 ':' <> encodeToBuilder v
        key = toAesonBuilder . Aeson.String . Aeson.toText

    scientific v 
      | v < 1 = S.formatScientificBuilder S.Fixed Nothing v
      | otherwise = S.formatScientificBuilder S.Fixed (Just 0) v

-- >>> encode' $ Aeson.object ["slot" .= (0.00000003 :: Float) ]
-- "{\"slot\":0.00000003}"
--
-- >>> Aeson.encode $ Aeson.object ["slot" .= (0.00000003 :: Float) ]
-- "{\"slot\":3.0e-8}"

-- >>> encode' $ Aeson.object ["slot" .= (maxBound :: Word64) ]
-- "{\"slot\":18446744073709551615}"

-- >>> Aeson.encode $ Aeson.object ["slot" .= (maxBound :: Word64) ]
-- "{\"slot\":18446744073709551615}"

TBD: where this encoder should be placed

rphair commented 1 month ago

thanks @carbolymer - as of node 9.2.1 with "ViewMode": "SimpleView" set in config.json, the slot number comes as integer or scientific notation depending on log facility:

Block number is also in scientific notation when logging successfully forged blocks, as well as the slot number (same as reported above already):

I do think it would be more readable and parseable to have chainDensity as an unexponented decimal, rather than scientific notation: especially since it will not be changing magnitude.