Clean up multiservice-discovery to be more robust, lock shared struct…

[DFINITYManu]

…ures, and separate concerns.

In particular, the use of parallel lists for thread end signaling channels and actual threads has been eliminated.

Additional changes:

• improved logging,
• improved messages issued to the user via HTTP,
• graceful interruption of initial syncs,
• parallel stop and start of multiple definitions submitted via POST / PUT,
• typing improvements (better error typing, structural typing to prevent usage bugs with the DefinitionSupervisor).

Preliminary work to ensure other modifications are possible.

[NikolaMilosa]

Nice! I like the distinction between running and non-running definitions. The only thing I am not a fan of are the comments. While they do help sometimes I feel like a lot of comments don't allow me to really understand the code but actually push me towards thinking that is what the code does without reviewing it. I am okay with some comments before the function which explains what the function on high level does but if we need comments in the flow I think we are just having bad names. If we added these comments above the functions we could add some sort of exporter which would generate docs for us and include it in our github pages in the future.

[DFINITYManu]

I can't fix the broken test. The test data is in protobuf format, and I can't tell what the structure is, so I can't tell what the output is supposed to look like.

EDIT: looks like the test is fixed, although it wasn't a good test before -- it used opaque data. We should avoid that.

EDIT: the test is still broken, and left a process running in the background.

test tests::prom_targets_tests ... FAILED

failures:

---- tests::prom_targets_tests stdout ----
thread 'tests::prom_targets_tests' panicked at rs/ic-observability/multiservice-discovery/tests/tests.rs:52:52:
called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
Jan 30 17:34:20.434 WARN Failed to sync registry for mercury @ interval Instant { tv_sec: 577545, tv_nsec: 925348376 }: SyncWithNnsFailed { failures: [("targets", RegistryTransportError { source: UnknownError("InvalidSignature(\"certificate_tree_hash=CryptoHash(0x544aeefb9e5adb7fe2f0ca2597c4d85eda942fdc488ac8b9a2f5421ff763e3b7), sig=Blob{48 bytes;b16fca4c1f0429733f4d22a0834d675e727f855c57090ce7906779344e5cc8fe518081db10b3cd9d9bc1084111fa23a1}, pk=ThresholdSigPublicKey { internal: ThresBls12_381(0x80da00812e71c6975396e30a2bf353e70a44361ee17e4c12ee9a5de5f2580d5b9c4cf7f4eb9d258b2a8cbdb95ecd143c16a7570d279e6e5de46fd5b463fa0b2462d77f0d8305e12f8719972c7bdb5ba4061d31f3a87745eddce5fa3ed52b87b3) }, error=ThresBls12_381 signature could not be verified: public key 80da00812e71c6975396e30a2bf353e70a44361ee17e4c12ee9a5de5f2580d5b9c4cf7f4eb9d258b2a8cbdb95ecd143c16a7570d279e6e5de46fd5b463fa0b2462d77f0d8305e12f8719972c7bdb5ba4061d31f3a87745eddce5fa3ed52b87b3, signature b16fca4c1f0429733f4d22a0834d675e727f855c57090ce7906779344e5cc8fe518081db10b3cd9d9bc1084111fa23a1, error: Invalid combined threshold signature\")") })] }

[DFINITYManu]

     Running tests/tests.rs (target/debug/deps/tests-b845cb7b0ebbef8d)

running 1 test
test tests::prom_targets_tests ... FAILED

failures:

---- tests::prom_targets_tests stdout ----
thread 'tests::prom_targets_tests' panicked at rs/ic-observability/multiservice-discovery/tests/tests.rs:52:52:
called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    tests::prom_targets_tests

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Can we delete this test and make a proper unit test instead?

[NikolaMilosa]

I believe this was supposed to be an integration test not a unit test. cc @pietrodimarco-dfinity