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

chore: logging received message info via onValidated observer #2973

Merged
merged 5 commits into from
Aug 19, 2024

Conversation

gabrielmer
Copy link
Contributor

@gabrielmer gabrielmer commented Aug 14, 2024

Description

Logging information related to received messages by using one of the new onValidated nim-libp2p observers. This is improves our current approach of logging via the traceHandler, which does not provide information on who sent the message a node received.

Changes

  • setting onValidated logger observer
  • removed message received logging from traceHandler
  • added message received log when a message is sent via REST, as per DST request
  • renamed initRelayMetricObserver to initRelayObservers

Notice that a "message received" log were added to the REST endpoints that publish messages. This is per DST's request, so they can easily tag the node that published a message to the network as a node that has seen the message.

CC @AlbertoSoutullo

Example

image

Copy link

github-actions bot commented Aug 14, 2024

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2973

Built from c3828f3

@gabrielmer gabrielmer marked this pull request as ready for review August 14, 2024 15:59
@AlbertoSoutullo
Copy link

To add a bit of context regarding added message received log when a message is sent via REST:
In order to check message reliability via logging, the problem we were facing when messages were being injected in the network via REST is that all nodes were logging the received message but the node that started the publishing. So we wanted a way to make sure that at least this node also "received" this message but from a different way, so we could make sure this happened, instead of assuming it, if it makes sense.

Copy link
Collaborator

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

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

Gorgeous one! Thanks so much for it! 💯

waku/waku_relay/protocol.nim Outdated Show resolved Hide resolved
@gabrielmer gabrielmer requested a review from SionoiS August 15, 2024 12:38
Copy link
Contributor

@SionoiS SionoiS left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

waku/waku_relay/protocol.nim Show resolved Hide resolved
@gabrielmer gabrielmer merged commit e8bce67 into master Aug 19, 2024
9 of 11 checks passed
@gabrielmer gabrielmer deleted the chore-log-message-hashes-with-observers branch August 19, 2024 12:13
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.

4 participants