Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simple HTML frontend for Bisq 2 monitor #2978

Merged

Conversation

bsc7
Copy link
Contributor

@bsc7 bsc7 commented Nov 5, 2024

Accessible via http://localhost:8082/node-monitor/index.html

The address list is automatically loaded via a REST interface (get-address-list). Seed node addresses are loaded from the configuration, while two oracle nodes are hardcoded.

The address list and port filter can be set in the settings as a list, separated by newlines or commas, and will be saved in a cookie. Comments (lines beginning with #) and empty lines are allowed.

Origin: #2634

Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK
Small Nit see in comment...

.toList();

CompletableFuture<Void> allFutures = CompletableFuture.allOf(futures.toArray(new CompletableFuture[0]));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a util method CompletableFutureUtils.allOf with an improved API (see comments in the method about the differences to CompletableFuture.allOf).

@HenrikJannsen
Copy link
Contributor

HenrikJannsen commented Nov 5, 2024

I tested it locally now. Looks great! A few small suggestions:

  • Can we remove version, serializedSize and excludedFields? Those are properties from the Proto class and not relevant here.
  • Could we display the operators user name (or if "N/A" the bond user name) and node type (Seed, Oracle) next to the address? Like: "Seed node operator: HenrikJannsen; Address: hj2kj43oyq4mhd5gx4vokalnci3vlbwzclv7usocfwuj5f5iks3eivqd.onion:1000" or the like...
  • Could we add a "Show diff" button and display all data from the Authorized Data Per Class Name, Authenticated Data Per Class Name and Mailbox Data Per Class Name groups which do not have a 100% match among all nodes? Maybe with some color to make it easier to spot them? This would help to see data which is not well distributed. It is expected due the nature of the P2P network that data is not always 100% the same but should be minimal if the network is healthy. As that might be a bigger change, feel free to do it in a new PR if you wish.

There was an inactive seed node still in the network. I banned it now and added below the code for filtering banned nodes.
Feel free to add it, then I don't need to do a new PR on top of yours...

public List<String> getAddressList() {
        Set<Address> bannedAddresses = bondedRolesService.getAuthorizedBondedRolesService().getBondedRoles().stream()
                .filter(BondedRole::isBanned)
                .map(BondedRole::getAuthorizedBondedRole)
                .map(AuthorizedBondedRole::getAddressByTransportTypeMap)
                .filter(Optional::isPresent)
                .map(Optional::get)
                .flatMap(map -> map.values().stream())
                .collect(Collectors.toSet());
        Map<TransportType, Set<Address>> seedAddressesByTransport = networkService.getSeedAddressesByTransportFromConfig();
        Set<TransportType> supportedTransportTypes = networkService.getSupportedTransportTypes();
        List<String> addresslist = seedAddressesByTransport.entrySet().stream()
                .filter(entry -> supportedTransportTypes.contains(entry.getKey()))
                .flatMap(entry -> entry.getValue().stream())
                .filter(address -> !bannedAddresses.contains(address))
                .map(Address::toString)
                .collect(Collectors.toList());

        // Oracle Nodes
        addresslist.add("kr4yvzlhwt5binpw7js2tsfqv6mjd4klmslmcxw3c5izsaqh5vvsp6ad.onion:36185");
        addresslist.add("s2yxxqvyofzud32mxliya3dihj5rdlowagkblqqtntxhi7cbdaufqkid.onion:54467");

        return addresslist;
    }

bsc7 added 4 commits November 5, 2024 12:49
Accessible via http://localhost:8082/node-monitor/index.html

The hostlist and portlist-filter can be set in settings as a list,
separated by newlines or commas, and will be saved in a cookie.
Filter out unnecessary fields (version, serializedSize, excludedFields) from reports.
@bsc7 bsc7 force-pushed the feature/2634/Simple_HTML_NodeMonitor branch from 912a334 to 2bf5039 Compare November 5, 2024 12:44
@bsc7
Copy link
Contributor Author

bsc7 commented Nov 5, 2024

Branch has been rebased(without conflicts) onto the latest main branch.


  • Could we display the operators user name...

I'll need to consider the best way to implement this, but overall, it shouldn't be a big issue.

  • Could we add a "Show diff" button...

This will require a bit more thought on my end :) but it should also be doable.

I'm still unsure whether I'll make a separate PR for these two points; it depends on how the timing works out.

@HenrikJannsen
Copy link
Contributor

  • Could we display the operators user name...

I'll need to consider the best way to implement this, but overall, it shouldn't be a big issue.

Here a code snippet how to get the data:

Set<String> nickNamesOrBondNames = bondedRolesService.getAuthorizedBondedRolesService().getBondedRoles().stream()
                .map(BondedRole::getAuthorizedBondedRole)
                .map(authorizedBondedRole -> userService.getUserProfileService().findUserProfile(authorizedBondedRole.getProfileId())
                        .map(UserProfile::getNickName)
                        .orElse(authorizedBondedRole.getBondUserName()))
                .collect(Collectors.toSet());
  • Could we add a "Show diff" button...

This will require a bit more thought on my end :) but it should also be doable.

I'm still unsure whether I'll make a separate PR for these two points; it depends on how the timing works out.

Yes feel free to do it as you wish. I also assume the diff feature might be a bit more work and would justify an additional PR.

Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@HenrikJannsen
Copy link
Contributor

Maybe better to merge the PR. There are a few other larger PRs and risk for merge conflict rises....

@HenrikJannsen HenrikJannsen merged commit bbf8623 into bisq-network:main Nov 7, 2024
15 checks passed
@bsc7 bsc7 deleted the feature/2634/Simple_HTML_NodeMonitor branch November 7, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants