Update monitor with last version from freimairs repo #4685
Conversation
The monitor does not use the common bisq app base featuring guice. Hence, the `config` in the `Connection` class is never injected and leads to NullPointerExceptions and ultimately breaks the monitor. This workaround initilizes default values in case guice isn't there.
solved in another PR which we will add to that PR in next commits)
chimp1984
deleted the
update-monitor-with-last-version-from-freimairs-repo
branch
|
ahm, guys, there is a reason why I did not create a PR for my local changes to the monitor:
so @chimp1984 you pretty much pulled in stuff created by other people without even contacting the people you pulled the stuff from and the reviewer @sqrrm did not even fire the thing up. That is not good practice! |
|
The only non-monitor change was in Connection which I reverted and is replaced by another better solution from another PR. |
that is not entirely true. You before all should know that. Back in the days we started monitoring stuff because we wanted to find out how healthy the Bisq network is. First task, for example, was to see if the Tor network is ok, so we can establish a baseline for Bisq's p2p network performance. Since then, we added metrics of areas which seemed interesting and we learned a whole lot about our p2p network. Being an alert system for the seed nodes has only been another use case that got added later. Sadly, we never managed to pull it off, simply because it turned out to be very hard to rule out false positives. We even tried binding compensation rates to seednode uptimes (https://monitor.bisq.network/d/G0Mk_uIWk/seednode-uptime-playground?orgId=1), however, these efforts stalled as well because nobody wanted to make a decision. The most recent addition has been the network size estimation which clearly showed us that although we expected thousands of users, bisq only has <400 at any time (meaning we have ~33 clients per seednode, and a seednode can host 50 - so Bisq's p2p network is actually hardly decentralized). Plus, in the last months, the initial data request sizes outgrew the capabilities of the monitors Tor uplink (because first, data sizes grew significantly, and second, we once had 6 seednodes, now we have 12) and hence, some metrics started glitching out more than I liked it (please be aware that I formulated and implemented a fix months ago which you recently took over). All in all, the monitor helped finding issues and design counter strategies. And that is what the monitor was created for - that is what I and others got paid for. The bad part is that the issues we found turned out to be very substantial and improving the situation requires deep changes in the p2p part of the code. Thus the counter strategies either never got merged or have been reverted shortly after being applied (the daily seednode restart prevails and seednodes are not synced up properly still), other changes where tagged a "hosting issue" while totally ignoring the data. |
Maybe you saw that this way, for me it was always the main goal. |
|
well, the monitoring project has been designed that way and accepted that way. So I did exactly that. Here is the accompanying proposal: bisq-network/proposals#62 |
Update monitor code base with latest version form @freimair repo.
Current version does not work due a nullpointer in connection. This was solved in @freimair's repo, but we removed it in that PR as it will have a better solution in #4680. We will make another new PR where this PR will be based on #4680 so the monitor become valid again.