experimental introspection support -- Project Phantom Drift PoC #964
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ctable Make routed host introspectable
This was referenced Jun 5, 2020
raulk
commented
Jun 5, 2020
| @@ -204,6 +220,7 @@ func NewHost(ctx context.Context, n network.Network, opts *HostOpts) (*BasicHost | |||
| h.mux = opts.MultistreamMuxer | |||
| } | |||
|
|
|||
| // Start ID Service | |||
raulk
commented
Jun 5, 2020
| maddrType = reflect.TypeOf(new(multiaddr.Multiaddr)).Elem() | ||
| ) | ||
|
|
||
| type eventManager struct { |
vyzo
reviewed
Jun 5, 2020
introspect/default_inspector.go
Outdated
|
|
||
| func (d *DefaultIntrospector) Close() error { | ||
| if err := d.wsub.Close(); err != nil { | ||
| return fmt.Errorf("failed while trying to close wildcard eventbus subscription: %w", err) |
There was a problem hiding this comment.
shouldn't we still close the rest of resources (and maybe collect a multierror)?
| Implementation: "go-libp2p", | ||
| Platform: runtime.GOOS, | ||
| PeerId: d.host.ID().Pretty(), | ||
| Version: "", |
There was a problem hiding this comment.
We don't really have a go-libp2p version/commit anyway to send back, as far as I'm aware?
Comment on lines
+63
to
+65
| close(em.closeCh) | ||
| em.closeWg.Wait() | ||
| return nil |
There was a problem hiding this comment.
shouldn't we also set em.closed to true?
There was a problem hiding this comment.
Yup. I have a TODO item to tackle closure idempotency. Will take care of that in a follow-up commit before merging.
|
@vyzo yup, this code is certainly not bug-free! It's marked as experimental, and these components are only used if the user enables the feature, so I think we're safe. |
|
Closing this PR. See #775 (comment) for motivation. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR represents a proof-of-concept for Project Phantom Drift (libp2p observability)
See #947 for more information.
The code being contributed herein is in EXPERIMENTAL status. It is relatively isolated under the
introspectpackage.Test coverage of that package is 82%.
Changes
This PR makes
BasicHostandRoutedHostimplementhost.IntrospectableHost.It contributes default implementations of:
introspect.DefaultIntrospector), responsible for introspecting the state of the system. It currently supports:ws.Endpoint), adhering to the protobuf introspection protocol in go-libp2p-core.Enabling introspection
Introspection MUST be enabled expressly, by using the
libp2p.Introspectionconstructor option:The actual listen address of the endpoint can be obtained via:
The BasicHost's constructor and
Closemethod take care of managing the lifecycle of theIntrospectorandIntrospectionEndpointobjects, if non-nil.This PR supersedes:
Taking it for a spin
Check out the REPL: https://github.com/libp2p/repl. These changes have been integrated in the REPL to enable demos.
TODO
IntrospectionEndpointrestartable.