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

fix: tracer: emit raw peer ids for compatibility with libp2p tracer #10271

Merged
merged 2 commits into from
Mar 23, 2023

Conversation

iand
Copy link
Contributor

@iand iand commented Feb 14, 2023

Related Issues

The pubsub tracer emits traces in JSON format or posts JSON documents to an opensearch endpoint. The traces are a combination of libp2p-pubsub traces and a custom LotusTraceEvent, intermixed in the JSON stream. The libp2p traces use []byte as the type for peer IDs which, in JSON, is base64 encoded.

The LotusTraceEvent type uses a string for peer IDs (prettified) which is emitted as-is in the JSON.

However clients have no sensible way to distinguish the two encodings, the stream consists of a mix of the two representations:

{"type":11,"peerID":"ACQIARIgTM4a29NqIGdalfumxVWLOB6hZ4xm0ybAT4giXOe8/G4=","timestamp":1676302327478329014,"graft": ... 
{"type":100,"peerID":"12D3KooWEzBS8UbFGKaHGcksuteyq363zdfHCNB9ZrEzuHbWTyQy","timestamp":1676302331378019221,"peerScore ...

If a client attempts to decode into a struct using a []byte field they will get the base64 decoded bytes of the libp2p peer IDs and the garbled base64 decoding of the pretty peer ID emitted in LotusTraceEvent

If they use a string field, they get the pretty peer ID from LotusTraceEvent and a base64 encoded string for the libp2p traces.

Since one supported client is ElasticSearch, there is not much scope for the user to decode the peer IDs properly.

Fix this by using []byte and raw peer IDs in the LotusTraceEvent to retain compatibility the pre-existing libp2p trace field type

Proposed Changes

Change LotusTraceEvent.PeerID and TraceEventPeerScore.PeerID types to []byte

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • [ x PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

@iand iand marked this pull request as ready for review February 14, 2023 17:19
@iand iand requested a review from a team as a code owner February 14, 2023 17:19
@iand
Copy link
Contributor Author

iand commented Feb 24, 2023

The itest failures seem unrelated

@magik6k magik6k merged commit 1022ac5 into filecoin-project:master Mar 23, 2023
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