docs(adr): ADR #010: Incentivized Testnet Monitoring

[rootulp]

Rendered

Prerequisites

Based on #901 and likely shouldn't be merged before it

[rootulp]

Hmm I can't edit labels or reviewers but tagging @sysrex @Wondertan @liamsi
This is my first ADR so I would definitely appreciate feedback on how to improve this one & future ADRs.

[codecov-commenter]

Codecov Report

Merging #922 (842ae78) into main (2cc8077) will decrease coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #922      +/-   ##
==========================================
- Coverage   58.53%   58.46%   -0.08%     
==========================================
  Files         132      132              
  Lines        7947     7947              
==========================================
- Hits         4652     4646       -6     
- Misses       2818     2822       +4     
- Partials      477      479       +2     

Impacted Files	Coverage Δ
header/verify.go	74.41% <0.00%> (-6.98%)	⬇️
header/core/listener.go	52.83% <0.00%> (-5.67%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[Wondertan]

LGTM!

Should we specify that this ADR is (Incentivised) Testnet specific instead of general monitoring? I.e. the document does not specify monitoring of a node for node operators but global monitoring for the network. We can also extend the document to have both cases.

Regarding the structure. All the sub-headers are questions with answers, and there is also a section for Open Questions, which are also questions with answers. I'm not saying that this should be fixed, but I don't understand the difference between those.

[Wondertan]

There is something weird with the bullet points, all of them seem to be "1", is it intended?

[Wondertan]

I like the progress we have overall and the clarifications we make. However, I am requesting changes due to incorrect statements, like "Node operators will lose the ability to monitor their own celestia-node", which are pointed out in the review comments.

[musalbas]

Something for the node team to think about: what would opting in look like? A command-line flag, or an interactive prompt?

[rootulp]

what would opting in look like? A command-line flag, or an interactive prompt?

With #907 this looks like a command-line flag: --metrics.endpoint. I think command-line flag is slightly preferable over interactive prompt because the latter is more cumbersome to write scripts against. Thoughts @Wondertan?

[musalbas]

My main concern is that if it's some obscure flag that people explicitly have to add, very few people will know about it and help share metrics with us, even if they're ok with it.

Is there a way to explicitly ask people if they want to share metrics or not? Maybe when you run init there can be an interactive prompt. If people want to run a script there can be a --noninteractive option or something to skip the prompt?

[Wondertan]

@musalbas, @rootulp, I have no strong preferences between either script running over node binary and doing a prompt or the binary itself doing this on init. Both are the same amount of work, and choice can be given to the implementor. Although, if we want to keep this telemetry setup for long in the mainnet, then having it as a part of the binary might be more convenient. Similar to how we hardcode bootstrappers per network we run, we may also hardcode our telemetry endpoint to which data will be sent if users (1) agree to send it, (2)enabled telemetry(agreeing might also enable it by default), and (3) don't set a custom endpoint. Well, including this in binary seems like more work now, but I have no strong preferences.

[Wondertan]

LGTM. Thank you for bringing this over the line.

The flags/prompts and script/in-binary agreement above still need to be resolved, but i don't think it is blocking the merge

[rootulp]

I also don't have strong opinions here. I'm okay with the interactive prompt if a --noninteractive option exists. I created #1003 to continue the conversation because it seems like an implementation detail that doesn't strictly block this ADR.

[liamsi]

Great job! Thanks for the patience and also shoutout to @Wondertan and @sysrex for feedback and reviews.