Vishal/add operator details #185

vishalchangrani · 2020-11-26T21:48:08Z

Quality of life improvement:
Adding an additional nodeinfo field when reporting the network_gossip_node_reachable to help distinguish nodes by their operators.

If a file in the following format is made available on the access node, then on startup the AN will read the file and add the extra nodeinfo details when reporting metrics.

{
"6c6af0933b710655ec553f4bead3b01c5e0a3ffd1194ee536efb926b356c54aa":"samsung",
"cb4262ba2d7f6d3a6ff0b9bc1d847259dee3cb843d08b833ef940535ccfe2ec0":"bisontrail",
...
...
}

The other alternative that I explored was to provide a mapping exclusively in Grafana. However that approach had two problems:

The mapping had to be provided for each panel separately.
It involved editing the dashboard json and inserting an additional snippet which I felt would make it cumbersome.

Example:

…dashboard

vishalchangrani · 2020-11-26T21:52:01Z

cmd/access/main.go

@@ -82,6 +83,7 @@ func main() {
 			flags.BoolVar(&logTxTimeToFinalizedExecuted, "log-tx-time-to-finalized-executed", false, "log transaction time to finalized and executed")
 			flags.BoolVar(&pingEnabled, "ping-enabled", false, "whether to enable the ping process that pings all other peers and report the connectivity to metrics")
 			flags.BoolVar(&retryEnabled, "retry-enabled", false, "whether to enable the retry mechanism at the access node level")
+			flags.StringVarP(&nodeInfoFile, "node-info-file", "", "", "full path to a json file which provides more details about nodes when reporting its reachability metrics")


e.g.
...
--node-info-file /data/nodeinfo.json
...

zhangchiqing · 2020-11-26T22:07:45Z

engine/access/ping/engine.go

-	e.metrics.NodeReachable(peer, reachable)
+	id := peer.ID()
+	reachable := e.pingAddress(id)
+	info := e.nodeInfo[id]


what if the id is not found in the node info map?

then the nodeinfo reported is empty "".

I thought of putting in something unknown but then settled for just letting it be empty.

zhangchiqing

Nice improvement 👍

Kay-Zee · 2020-11-30T18:25:51Z

engine/access/ping/node_info.go

+	"github.com/onflow/flow-go/model/flow"
+)
+
+func readJson(jsonFileName string) (map[flow.Identifier]string, error) {


These are much more specialized json reading functions, lets give them more specific names, maybe readExtraNodeInfoJSON or similar. Same with the rest of the functions in this file. (Also, go convention prefers captializing the whole acronym, e.g. JSON instead of Json)

Kay-Zee · 2020-11-30T18:27:40Z

engine/access/ping/engine.go

+			eng.nodeInfo = nodeInfo
+			log.Debug().Str("node_info_file", nodeInfoFile).Msg("using node info file")
+		}
+	} else {


I noticed we don't initialize nodeInfo as an empty map. Will this cause any nil pointer issues for cases when we don't pass in a file?

The diff contains: - a lot of changes that concern parts of the library we don't use (other curves a la BN, BLS24-X, BLS12-383 ...), integer protocols (ETRS), field extension machinery ... - otherwise irrelevant changes, e.g. CI/CD - some memory bug fixing [Full Changeset](https://github.com/relic-toolkit/relic/compare/7a9bba7f..9206ae5) **Fixed bugs:** - Unexpected failure of ep2\_mul\[\_lwnaf\] above the prime group order [\onflow#64](relic-toolkit/relic#64) **Closed issues:** - Other way to construct towered extension fields [\onflow#203](relic-toolkit/relic#203) - blake2.h:101:5: error: size of array element is not a multiple of its alignment [\onflow#202](relic-toolkit/relic#202) - ECIES 160bit [\onflow#201](relic-toolkit/relic#201) - Compilation with "ARITH gmp" fails [\onflow#200](relic-toolkit/relic#200) - Support for armv8-a ? [\onflow#198](relic-toolkit/relic#198) - Function name bn\_init conflicts with OpenSSL when used in tandem [\onflow#196](relic-toolkit/relic#196) - 16-bit MSP430 [\onflow#193](relic-toolkit/relic#193) - Modular exponentiation returns 1 if exponent is 0 and modulo is 1 [\onflow#185](relic-toolkit/relic#185) - Compilation of RELIC with bls12-446 and bls12-455 fails [\onflow#182](relic-toolkit/relic#182) - test\_bn fails with BLS12-381 preset [\onflow#181](relic-toolkit/relic#181) - \[BUG\] undefined reference to `bench_init', `bench\_clean' [\onflow#180](relic-toolkit/relic#180) - Tests FTBFS because of missing symbol in header [\onflow#179](relic-toolkit/relic#179) - Builds are broken [\onflow#178](relic-toolkit/relic#178) - compile error inlining failed in call to always\_inline ‘\_mm\_alignr\_epi8’ on unbantu20.04 gcc9 [\onflow#177](relic-toolkit/relic#177) - bn\_write\_str buffer overflow [\onflow#176](relic-toolkit/relic#176) - ECDSA verify succeeds when it should fail [\onflow#175](relic-toolkit/relic#175) - ec\_mul\_gen hangs with curve SECG\_K256 [\onflow#174](relic-toolkit/relic#174) - Wrong square root computation [\onflow#173](relic-toolkit/relic#173) - Out-of-bounds read via bn\_sqr\_basic [\onflow#172](relic-toolkit/relic#172) - OSS-Fuzz integration [\onflow#171](relic-toolkit/relic#171) - Building Relic with Curve NIST\_P256 throws FATAL ERROR in relic\_fp\_prime.c:120 [\onflow#170](relic-toolkit/relic#170) - Compressing \(packing\) a point to binary array does not comply with X9.62 standard [\onflow#169](relic-toolkit/relic#169) - ‘ctx\_t’ {aka ‘struct \_ctx\_t’} has no member named ‘total’ [\onflow#168](relic-toolkit/relic#168) - relic does not work with C++ [\onflow#167](relic-toolkit/relic#167) - Memory leak in ep2\_curve\_init/clean with ALLOC=DYNAMIC [\onflow#166](relic-toolkit/relic#166) - \*\_is\_valid\(\) functions produce false negative for not normalized points [\onflow#147](relic-toolkit/relic#147) - Bench and Test doesnt build [\onflow#122](relic-toolkit/relic#122) **Merged pull requests:** - Add pairing delegation protocols [\onflow#199](relic-toolkit/relic#199) ([dfaranha](https://github.com/dfaranha)) - Fix support for Win64/MSVC targets. [\onflow#197](relic-toolkit/relic#197) ([dfaranha](https://github.com/dfaranha)) - Simplify generator getting for Gt. [\onflow#194](relic-toolkit/relic#194) ([luozejiaqun](https://github.com/luozejiaqun)) - cmake: Always use user defined CFLAGS, not only for release builds [\onflow#187](relic-toolkit/relic#187) ([xdustinface](https://github.com/xdustinface)) - Fix MinGW build [\onflow#186](relic-toolkit/relic#186) ([xdustinface](https://github.com/xdustinface)) - Remove debug printf in bn\_mxp\_slide [\onflow#184](relic-toolkit/relic#184) ([guidovranken](https://github.com/guidovranken)) - Remove ALLOC = STACK to simplify memory allocation. [\onflow#183](relic-toolkit/relic#183) ([dfaranha](https://github.com/dfaranha)) - Update relic\_alloc.h [\onflow#165](relic-toolkit/relic#165) ([aguycalled](https://github.com/aguycalled)) - Add correct support for FreeBSD and NetBSD [\onflow#164](relic-toolkit/relic#164) ([hoffmang9](https://github.com/hoffmang9))

vishalchangrani added 2 commits November 25, 2020 17:58

adding nodeinfo to ping metrics to help identify operator on Grafana …

4436c42

…dashboard

adding node-info-file command line argument for access node

88a08b8

vishalchangrani requested review from zhangchiqing, ramtinms and Kay-Zee November 26, 2020 21:48

vishalchangrani self-assigned this Nov 26, 2020

vishalchangrani modified the milestone: 27 Nov 26, 2020

vishalchangrani commented Nov 26, 2020

View reviewed changes

zhangchiqing reviewed Nov 26, 2020

View reviewed changes

vishalchangrani requested review from AlexHentschel, jordanschalm, m4ksio, psiemens and yhassanzadeh13 as code owners November 27, 2020 21:48

vishalchangrani requested review from ramtinms and removed request for m4ksio, ramtinms, psiemens, jordanschalm, yhassanzadeh13 and AlexHentschel November 27, 2020 21:49

zhangchiqing approved these changes Nov 27, 2020

View reviewed changes

Merge branch 'master' into vishal/add_operator_details

820fd58

vishalchangrani force-pushed the vishal/add_operator_details branch from 261dbb4 to 820fd58 Compare November 28, 2020 00:34

Kay-Zee reviewed Nov 30, 2020

View reviewed changes

vishalchangrani added 4 commits November 30, 2020 11:06

review comments (commit will be squashed)

b393853

review comments (commit will be squashed)

aed0949

Merge branch 'master' into vishal/add_operator_details

99b7e4b

Merge branch 'master' into vishal/add_operator_details

1514f29

Merge branch 'master' into vishal/add_operator_details

ae11607

Kay-Zee approved these changes Dec 5, 2020

View reviewed changes

vishalchangrani merged commit f5a4097 into master Dec 5, 2020

vishalchangrani deleted the vishal/add_operator_details branch December 5, 2020 03:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vishal/add operator details #185

Vishal/add operator details #185

vishalchangrani commented Nov 26, 2020 •

edited

Loading

vishalchangrani Nov 26, 2020

zhangchiqing Nov 26, 2020

vishalchangrani Nov 26, 2020

vishalchangrani Nov 26, 2020

zhangchiqing left a comment

Kay-Zee Nov 30, 2020

Kay-Zee Nov 30, 2020

Vishal/add operator details #185

Vishal/add operator details #185

Conversation

vishalchangrani commented Nov 26, 2020 • edited Loading

vishalchangrani Nov 26, 2020

Choose a reason for hiding this comment

zhangchiqing Nov 26, 2020

Choose a reason for hiding this comment

vishalchangrani Nov 26, 2020

Choose a reason for hiding this comment

vishalchangrani Nov 26, 2020

Choose a reason for hiding this comment

zhangchiqing left a comment

Choose a reason for hiding this comment

Kay-Zee Nov 30, 2020

Choose a reason for hiding this comment

Kay-Zee Nov 30, 2020

Choose a reason for hiding this comment

vishalchangrani commented Nov 26, 2020 •

edited

Loading