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

feat(telemetry): Add url data to operator updates on telemetry service #1136

Merged
merged 70 commits into from
Oct 16, 2024

Conversation

JulianVentura
Copy link
Collaborator

@JulianVentura JulianVentura commented Oct 1, 2024

Description

This PR adds new data to be updated for each operator on the telemetry service, which includes:

  • eth_rpc_url
  • eth_rpc_url_fallback
  • eth_ws_url
  • eth_ws_url_fallback
  • version (this one was already present)

The implementation is also updated from the operator's side.

Testing

In order to test, the following steps were taken:

  1. Start anvil and the aggregator with:
  • make anvil_start_with_block_time
  • make aggregator_start
  1. Build and register operator with:
  • make build_operator
  • make operator_full_registration CONFIG_FILE=config-files/config-operator-1.yaml
  1. Start telemetry service with:
  • make telemetry_clean_db
  • make telemetry_run_db
  • make telemetry_start
  1. Go to http://localhost:4001/api/operators and you should see the operator with its data fields. Some of them, like the version and RPC related should be set as null, since the operator is not up yet.
  2. Start the operator with ./operator/build/aligned-operator start --config ./config-files/config-operator-1.yaml
  3. Go to http://localhost:4001/api/operators and you should see the operator, now with all fields with a certain value, including version and RPC related.

It is also highly advisable to test previous functionality as it's described on this already merged PR

JuArce and others added 21 commits September 18, 2024 14:11
feat: CRUD for operators version
docs: update README
…-in-elixir' into 1075-feattelemetry-implement-telemetry-for-aggregator

# Conflicts:
#	telemetry_api/config/dev.exs
#	telemetry_api/mix.exs
#	telemetry_api/mix.lock
…-in-elixir' into 1075-feattelemetry-implement-telemetry-for-aggregator
fix: store address and operator_id in lowercase
…-in-elixir' into 1075-feattelemetry-implement-telemetry-for-aggregator
@JulianVentura JulianVentura marked this pull request as ready for review October 1, 2024 21:14
@JulianVentura JulianVentura marked this pull request as draft October 1, 2024 21:42
@JulianVentura JulianVentura marked this pull request as ready for review October 2, 2024 14:54
operator/cmd/actions/start.go Outdated Show resolved Hide resolved
operator/cmd/actions/start.go Outdated Show resolved Hide resolved
Base automatically changed from 1075-feattelemetry-implement-telemetry-for-aggregator to staging October 10, 2024 16:12
Copy link

github-actions bot commented Oct 14, 2024

Changes to gas cost

Generated at commit: 1ba65813a42630c266a2fba6c6607917396efff6, compared to commit: 07f682c7f5bab0ef2711f7373ba865d4ef9d5258

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
AlignedLayerServiceManager createNewTask -24 ✅ -0.03%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
AlignedLayerServiceManager 4,681,797 (0) createNewTask 53,859 (+24) +0.04% 73,818 (-24) -0.03% 73,963 (-18) -0.02% 74,821 (+36) +0.05% 256 (0)
RegistryCoordinatorHarness 5,846,028 (+17,275)

Copy link
Collaborator

@MarcosNicolau MarcosNicolau left a comment

Choose a reason for hiding this comment

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

Tested on my machine, and it worked fine. One thing, in the diff I see you changed the eigenlayer-middleware, is that on purpose?

@JulianVentura
Copy link
Collaborator Author

Tested on my machine, and it worked fine. One thing, in the diff I see you changed the eigenlayer-middleware, is that on purpose?

No, I haven't touched anything related to that. There was a merge from staging branch, so maybe it was an auto merge error? eigenlayer-middleware is a submodule, I checked and the staging branch points to the same commit as this one, so Idk what caused that diff.

@JulianVentura
Copy link
Collaborator Author

Tested on my machine, and it worked fine. One thing, in the diff I see you changed the eigenlayer-middleware, is that on purpose?

No, I haven't touched anything related to that. There was a merge from staging branch, so maybe it was an auto merge error? eigenlayer-middleware is a submodule, I checked and the staging branch points to the same commit as this one, so Idk what caused that diff.

My bad. The submodule commit was in fact different from the one in staging branch. Good catch!

@entropidelic
Copy link
Contributor

entropidelic commented Oct 15, 2024

everything worked correctly, but I am thinking that it does not make much sense to register the http and websocket providers URL separately. Since the protocol and the API key are stripped from the URL, they will always be the same, or am I missing something?
nvm, talked with @JuArce and the current behaviour is correct

operator/pkg/utils.go Outdated Show resolved Hide resolved
operator/pkg/utils_test.go Show resolved Hide resolved
Copy link
Contributor

@uri-99 uri-99 left a comment

Choose a reason for hiding this comment

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

Meant to request changes because of my previous comments. Everything else works

operator/cmd/actions/utils.go Outdated Show resolved Hide resolved
operator/cmd/actions/start.go Outdated Show resolved Hide resolved
operator/cmd/actions/start.go Outdated Show resolved Hide resolved
@JuArce JuArce merged commit 8164a8f into staging Oct 16, 2024
2 checks passed
@JuArce JuArce deleted the feat/telemetry-operator-url-data branch October 16, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

7 participants