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

Add grpc endpoint to EN for transaction execution metrics #6172

Merged
merged 14 commits into from
Sep 11, 2024

Conversation

janezpodhostnik
Copy link
Contributor

@janezpodhostnik janezpodhostnik commented Jul 3, 2024

ref: #5598

this adds a grpc endpoint that can be called to get metrics around transaction execution. The metrics include:

  • block height
  • tx id
  • tx execution time in nanoseconds
  • tx execution effort breakdown by computation kind

@janezpodhostnik janezpodhostnik self-assigned this Jul 3, 2024
@janezpodhostnik janezpodhostnik changed the base branch from janez/add-transaction-info-to-metrics to master July 3, 2024 15:48
@janezpodhostnik janezpodhostnik force-pushed the janez/expose-transaction-metrics branch from a684743 to 31f1186 Compare July 3, 2024 17:44
@codecov-commenter
Copy link

codecov-commenter commented Jul 3, 2024

Codecov Report

Attention: Patch coverage is 34.61538% with 170 lines in your changes missing coverage. Please review.

Project coverage is 41.41%. Comparing base (5e2d76f) to head (2464067).
Report is 160 commits behind head on master.

Files with missing lines Patch % Lines
...mputation/metrics/transaction_execution_metrics.go 0.00% 46 Missing ⚠️
engine/execution/rpc/engine.go 0.00% 31 Missing ⚠️
cmd/execution_builder.go 0.00% 28 Missing ⚠️
engine/access/mock/execution_api_client.go 0.00% 24 Missing ⚠️
engine/access/mock/execution_api_server.go 0.00% 18 Missing ⚠️
engine/execution/computation/metrics/collector.go 77.77% 11 Missing and 1 partial ⚠️
module/metrics/execution.go 0.00% 7 Missing ⚠️
cmd/execution_config.go 0.00% 2 Missing ⚠️
engine/execution/computation/metrics/provider.go 96.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6172      +/-   ##
==========================================
- Coverage   41.42%   41.41%   -0.01%     
==========================================
  Files        2021     2024       +3     
  Lines      143934   144193     +259     
==========================================
+ Hits        59620    59716      +96     
- Misses      78142    78303     +161     
- Partials     6172     6174       +2     
Flag Coverage Δ
unittests 41.41% <34.61%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@janezpodhostnik janezpodhostnik force-pushed the janez/expose-transaction-metrics branch from 31f1186 to 7131993 Compare July 3, 2024 18:59
@janezpodhostnik janezpodhostnik force-pushed the janez/expose-transaction-metrics branch from 7131993 to 5f0dfa9 Compare July 23, 2024 13:34
@janezpodhostnik janezpodhostnik marked this pull request as ready for review July 23, 2024 19:14
@janezpodhostnik janezpodhostnik requested a review from a team July 23, 2024 19:15
@janezpodhostnik janezpodhostnik changed the title Added transaction information to transaction execution metrics Add grpc endpoint to EN for transaction execution metrics Jul 23, 2024
engine/execution/rpc/engine.go Show resolved Hide resolved
engine/execution/computation/metrics/collector.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/collector_test.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/collector_test.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/provider.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/provider.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/provider.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/provider.go Outdated Show resolved Hide resolved
module/metrics/execution.go Outdated Show resolved Hide resolved
Copy link
Contributor

@peterargue peterargue left a comment

Choose a reason for hiding this comment

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

looks good. mostly testing and stylistic comments

module/metrics/execution.go Outdated Show resolved Hide resolved
cmd/execution_builder.go Outdated Show resolved Hide resolved
cmd/execution_builder.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/collector.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/collector.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/collector.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/collector.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/provider.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/provider.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/provider.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/provider.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/collector.go Outdated Show resolved Hide resolved
engine/execution/computation/metrics/collector.go Outdated Show resolved Hide resolved
@janezpodhostnik janezpodhostnik added this pull request to the merge queue Sep 11, 2024
Merged via the queue into master with commit 9653906 Sep 11, 2024
55 checks passed
@janezpodhostnik janezpodhostnik deleted the janez/expose-transaction-metrics branch September 11, 2024 19:31
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.

5 participants