Skip to content

feat: tasks executor metrics in grafana #11815

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.

Sign up for GitHub

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

Jump to bottom
Merged
mattsse merged 5 commits into from
Oct 18, 2024
Merged

feat: tasks executor metrics in grafana #11815

mattsse merged 5 commits into from
Oct 18, 2024

Conversation

@greged93
Copy link
Contributor

@greged93 greged93 commented Oct 16, 2024

Adds the TaskExecutorMetrics to Grafana. Resolves #8332.

Rjected
Rjected requested changes Oct 16, 2024
View reviewed changes
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

Do you mind adding a screenshot to the PR description?

You also need to do "Export for sharing externally", the UIDs need to be a variable:
Screenshot 2024-10-16 at 6 01 52 PM

@greged93
Copy link
Contributor Author

greged93 commented Oct 16, 2024

sure! here is the screenshot for the two dashboards:
Screenshot 2024-10-17 at 00 05 25

@greged93
Copy link
Contributor Author

greged93 commented Oct 16, 2024

forgot to mention that I decided not to have the rate for the critical tasks, as this was not very interesting as there is very little change in the amount of critical tasks running.

Rjected
Rjected approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@Rjected Rjected 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!

@onbjerg
Copy link
Collaborator

onbjerg commented Oct 17, 2024

I'm not sure I understand the second panel, how is it stopping -2 tasks per second?

@onbjerg onbjerg added the A-observability Related to tracing, metrics, logs and other observability tools label Oct 17, 2024
Rjected
Rjected requested changes Oct 17, 2024
View reviewed changes
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

I'm not sure I understand the second panel, how is it stopping -2 tasks per second?

Actually yeah, looking at it a second time, I don't think we should use the rate this way. Let's just use start, since they seem to just be the negative of each other?

onbjerg
onbjerg reviewed Oct 17, 2024
View reviewed changes
Copy link
Collaborator

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

in the same go as @Rjected's comment, let's also rename the panels to have "tasks" in them, e.g. "Task executor critical tasks", "Task executor regular tasks":)

@greged93
Copy link
Contributor Author

greged93 commented Oct 17, 2024

Will update! This was done because the issue itself mentioned adding the same metrics as the Tx and Rx messages, which use the rate

@greged93
Copy link
Contributor Author

greged93 commented Oct 17, 2024 •
edited
Loading

since they seem to just be the negative of each other?

They are very close to each other, I think the diff in the rates when I was looking was usually round ~0.2 tasks/s at start up.

@greged93 greged93 force-pushed the greged93/feat/grafana-task-executor branch from 1f5991b to 8422df6 Compare October 18, 2024 07:59
@greged93
Copy link
Contributor Author

greged93 commented Oct 18, 2024

New screenshot
Screenshot 2024-10-18 at 10 05 13

Rjected
Rjected requested changes Oct 18, 2024
View reviewed changes
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

only one nitpick, otherwise this looks good!

@greged93 @Rjected
Update overview.json
7d04e71 
Co-authored-by: Dan Cline <6798349+Rjected@users.noreply.github.com>
Rjected
Rjected approved these changes Oct 18, 2024
View reviewed changes
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

lgtm!

@mattsse mattsse added this pull request to the merge queue Oct 18, 2024
Merged via the queue into paradigmxyz:main with commit a4126b3 Oct 18, 2024
liamaharon added a commit to liamaharon/reth that referenced this pull request Oct 19, 2024
@liamaharon
fix: feature propagation
0971015 
Optimize Sender Recovery Process (paradigmxyz#11385)

chore: simplify update fn (paradigmxyz#11880)

bump rust to 1.82 (paradigmxyz#11876)

rpc: add unit tests for `RpcModuleSelection` (paradigmxyz#11883)

feat: tasks executor metrics in grafana (paradigmxyz#11815)

Co-authored-by: Dan Cline <6798349+Rjected@users.noreply.github.com>

refactor(txpool): small refactor for `InMemoryBlobStore` impl (paradigmxyz#11886)

primitives: use alloy `MAXIMUM_EXTRA_DATA_SIZE` constant (paradigmxyz#11881)

test(txpool): add unit test for `BlobStoreCanonTracker` (paradigmxyz#11885)

refactor: move `EngineValidator` setup to `RpcAddOns` (paradigmxyz#11850)

feat: update el requests for devnet 4 (paradigmxyz#11865)

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

check more features
liamaharon added a commit to liamaharon/reth that referenced this pull request Oct 19, 2024
@liamaharon
Squashed merge of the following commits:
cf94e9e 
commit 1a1aa2f
Author: Matthias Seitz <matthias.seitz@outlook.de>
Date:   Sat Oct 19 15:18:20 2024 +0200

    feat: add map_pool fn (paradigmxyz#11890)

commit ddc5ac3
Author: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Date:   Sat Oct 19 15:12:28 2024 +0200

    refactor(rpc): small refactor in `trace_filter` (paradigmxyz#11894)

commit 3793b90
Author: Matthias Seitz <matthias.seitz@outlook.de>
Date:   Sat Oct 19 15:05:53 2024 +0200

    chore: better start finish persisted block logs (paradigmxyz#11893)

    Co-authored-by: Oliver <onbjerg@users.noreply.github.com>

commit 3bd695e
Author: Oliver <onbjerg@users.noreply.github.com>
Date:   Sat Oct 19 14:48:35 2024 +0200

    feat: update el requests for devnet 4 (paradigmxyz#11865)

    Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

commit 2ae9368
Author: Arsenii Kulikov <klkvrr@gmail.com>
Date:   Sat Oct 19 14:08:34 2024 +0400

    refactor: move `EngineValidator` setup to `RpcAddOns` (paradigmxyz#11850)

commit da5079d
Author: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Date:   Sat Oct 19 10:28:42 2024 +0200

    test(txpool): add unit test for `BlobStoreCanonTracker` (paradigmxyz#11885)

commit 2f559c6
Author: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Date:   Sat Oct 19 10:28:10 2024 +0200

    primitives: use alloy `MAXIMUM_EXTRA_DATA_SIZE` constant (paradigmxyz#11881)

commit a6daafc
Author: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Date:   Sat Oct 19 10:27:29 2024 +0200

    refactor(txpool): small refactor for `InMemoryBlobStore` impl
(paradigmxyz#11886)

commit a4126b3
Author: greged93 <82421016+greged93@users.noreply.github.com>
Date:   Sat Oct 19 00:15:08 2024 +0200

    feat: tasks executor metrics in grafana (paradigmxyz#11815)

    Co-authored-by: Dan Cline <6798349+Rjected@users.noreply.github.com>

commit 655fc1a
Author: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Date:   Sat Oct 19 00:13:02 2024 +0200

    rpc: add unit tests for `RpcModuleSelection` (paradigmxyz#11883)

commit eee5e0d
Author: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Date:   Fri Oct 18 22:08:09 2024 +0200

    bump rust to 1.82 (paradigmxyz#11876)

commit a908f97
Author: Matthias Seitz <matthias.seitz@outlook.de>
Date:   Fri Oct 18 20:21:55 2024 +0200

    chore: simplify update fn (paradigmxyz#11880)

commit 587c91f
Author: Ayodeji Akinola <akinayodeji4all@gmail.com>
Date:   Fri Oct 18 16:17:11 2024 +0100

    Optimize Sender Recovery Process (paradigmxyz#11385)
Merge branch 'main' into fix-feature-propagation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@onbjerg onbjerg onbjerg left review comments

@Rjected Rjected Rjected approved these changes

@mattsse mattsse mattsse approved these changes

@shekhirin shekhirin Awaiting requested review from shekhirin shekhirin is a code owner

Assignees

No one assigned

Labels

A-observability Related to tracing, metrics, logs and other observability tools

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Integrate critical task metrics in grafana dashboard

4 participants

@greged93 @onbjerg @Rjected @mattsse