Added new flag + readme for starting p2p prometheus dashboards #5707
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
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## rc/v1.7.0 #5707 +/- ##
=============================================
- Coverage 80.32% 80.31% -0.01%
=============================================
Files 709 709
Lines 93901 93914 +13
=============================================
+ Hits 75425 75431 +6
- Misses 13185 13191 +6
- Partials 5291 5292 +1 ☔ View full report in Codecov by Sentry. |
raduchis
requested changes
Nov 14, 2023
facade/nodeFacade.go
Outdated
| @@ -36,7 +36,8 @@ import ( | |||
| const DefaultRestInterface = "localhost:8080" | |||
|
|
|||
| // DefaultRestPortOff is the default value that should be passed if it is desired | |||
| // to start the node without a REST endpoint available | |||
| // | |||
| // to start the node without a REST endpoint available | |||
facade/nodeFacade.go
Outdated
| @@ -163,7 +164,8 @@ func (nf *nodeFacade) RestAPIServerDebugMode() bool { | |||
|
|
|||
| // RestApiInterface returns the interface on which the rest API should start on, based on the config file provided. | |||
| // The API will start on the DefaultRestInterface value unless a correct value is passed or | |||
| // the value is explicitly set to off, in which case it will not start at all | |||
| // | |||
| // the value is explicitly set to off, in which case it will not start at all | |||
raduchis
reviewed
Nov 14, 2023
| @@ -17,14 +17,14 @@ func TestInitialNodeFacade(t *testing.T) { | |||
| t.Run("nil status metrics should error", func(t *testing.T) { | |||
| t.Parallel() | |||
|
|
|||
| inf, err := NewInitialNodeFacade("127.0.0.1:8080", true, nil) | |||
| inf, err := NewInitialNodeFacade("127.0.0.1:8080", true, false, nil) | |||
There was a problem hiding this comment.
maybe move all arguments for the NewInitialNodeFacade into a single arg-struct
README.md
Outdated
| 1. Start the node with `--p2p-prometheus-metrics` flag. This exposes a metrics collection at http://localhost:5001/debug/metrics/prometheus. | ||
| 2. Clone libp2p repository: `git clone https://github.com/libp2p/go-libp2p` | ||
| 3. `cd go-libp2p/dasboards/swarm` and under the | ||
| ```json |
There was a problem hiding this comment.
might remove the json tag because the code is not json compatible (missing ], } at the end)
Valid for L95. This will improve the readability in github
| @@ -73,6 +73,7 @@ GLOBAL OPTIONS: | |||
| --logs-path directory This flag specifies the directory where the node will store logs. | |||
| --operation-mode operation mode String flag for specifying the desired operation mode(s) of the node, resulting in altering some configuration values accordingly. Possible values are: snapshotless-observer, full-archive, db-lookup-extension, historical-balances or `""` (empty). Multiple values can be separated via , | |||
| --repopulate-tokens-supplies Boolean flag for repopulating the tokens supplies database. It will delete the current data, iterate over the entire trie and add he new obtained supplies | |||
| --p2p-prometheus-metrics Boolean option for enabling the /debug/metrics/prometheus route for p2p prometheus metrics | |||
README.md
Outdated
| @@ -83,6 +83,36 @@ sudo cp protoc-gen-gogoslick /usr/bin/ | |||
|
|
|||
| Done | |||
|
|
|||
| ## Running p2p Prometheus dashboards | |||
| 1. Start the node with `--p2p-prometheus-metrics` flag. This exposes a metrics collection at http://localhost:5001/debug/metrics/prometheus. | |||
There was a problem hiding this comment.
the port is not 5001, is the one defined by the -rest-api-interface flag which defaults to localhost:8080
README.md
Outdated
| ``` | ||
| sudo docker compose -f docker-compose.base.yml -f docker-compose-linux.yml up --force-recreate | ||
| ``` | ||
| **Note:** this command is not compatible with compose v1, thus an update to v2 would be needed. More details about the migration [here](https://docs.docker.com/compose/migrate/). |
There was a problem hiding this comment.
Let's add here: if you choose to install the new Docker version manually, please make sure that installation is done for all users of the system. Otherwise, the docker command will fail because it needs the super-user privileges.
... or something like this :)
raduchis
approved these changes
Nov 16, 2023
iulianpascalau
approved these changes
Nov 16, 2023
gabi-vuls
approved these changes
Nov 22, 2023
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.
Reasoning behind the pull request
Proposed changes
/debug/metrics/prometheusTesting procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?