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

Log summary about the node status #3870

Merged
merged 20 commits into from Jun 9, 2023
Merged

Conversation

millken
Copy link
Contributor

@millken millken commented May 29, 2023

Description

The node stats information will be printed to stdout every 5 minutes.

During indexer sync, stats report are not available

image Fixes #3782

Type of change

Please delete options that are not relevant.

  • [] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code refactor or improvement
  • [] Breaking change (fix or feature that would cause a new or changed behavior of existing functionality)
  • [] This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • [] make test
  • [] fullsync
  • [] Other test (please specify)

Test Configuration:

  • Firmware version:
  • Hardware:
  • Toolchain:
  • SDK:

Checklist:

  • [] My code follows the style guidelines of this project
  • [] I have performed a self-review of my code
  • [] I have commented my code, particularly in hard-to-understand areas
  • [] I have made corresponding changes to the documentation
  • [] My changes generate no new warnings
  • [] I have added tests that prove my fix is effective or that my feature works
  • [] New and existing unit tests pass locally with my changes
  • [] Any dependent changes have been merged and published in downstream modules

@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Merging #3870 (2931a12) into master (e1f0636) will increase coverage by 0.02%.
The diff coverage is 63.75%.

❗ Current head 2931a12 differs from pull request most recent head 5611262. Consider uploading reports for the commit 5611262 to get more accurate results

@@            Coverage Diff             @@
##           master    #3870      +/-   ##
==========================================
+ Coverage   75.38%   75.40%   +0.02%     
==========================================
  Files         303      328      +25     
  Lines       25923    27806    +1883     
==========================================
+ Hits        19541    20967    +1426     
- Misses       5360     5767     +407     
- Partials     1022     1072      +50     
Impacted Files Coverage Δ
action/protocol/execution/evm/evm.go 43.52% <0.00%> (-2.95%) ⬇️
action/protocol/execution/evm/evmstatedbadapter.go 66.77% <ø> (ø)
action/protocol/poll/consortium.go 0.00% <0.00%> (ø)
action/protocol/poll/staking_committee.go 43.85% <0.00%> (ø)
...tion/protocol/staking/contractstake_bucket_type.go 0.00% <0.00%> (ø)
api/grpcserver.go 86.40% <0.00%> (ø)
api/loglistener.go 25.00% <0.00%> (ø)
api/web3server_marshal.go 93.21% <ø> (ø)
api/websocket.go 5.17% <0.00%> (ø)
blockchain/config.go 74.54% <ø> (ø)
... and 49 more

... and 9 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@millken millken marked this pull request as ready for review May 30, 2023 07:46
@millken millken changed the title Log summary about node status Log summary about the node status May 30, 2023
@@ -156,6 +157,9 @@ type (
gasLimit uint64,
data []byte,
config *logger.Config) ([]byte, *action.Receipt, *logger.StructLogger, error)

// Track adds a track record for the given method
Track(ctx context.Context, start time.Time, method string, size int64, success bool)
Copy link
Member

Choose a reason for hiding this comment

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

why is an API added in the coreservice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is passed through the Option of newCoreService. There is already coreService in web3Handler, we can use it directly.

dispatcher/dispatcher.go Outdated Show resolved Hide resolved
api/types/types.go Show resolved Hide resolved
pkg/nodestats/nodestats.go Outdated Show resolved Hide resolved
pkg/nodestats/nodestats.go Outdated Show resolved Hide resolved
pkg/nodestats/nodestats.go Outdated Show resolved Hide resolved
pkg/nodestats/nodestats.go Outdated Show resolved Hide resolved
pkg/generic/map.go Outdated Show resolved Hide resolved
)
defer func(start time.Time) { svr.coreService.Track(ctx, start, method.(string), int64(size), err == nil) }(time.Now())
Copy link
Member

Choose a reason for hiding this comment

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

Do gRPC and HTTP servers not require invoking the Track method?

blocksync blocksync.BlockSync
p2pAgent p2p.Agent
task *routine.RecurringTask
}
Copy link
Member

@dustinxie dustinxie Jun 6, 2023

Choose a reason for hiding this comment

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

need to move to a diff place (should be server\itx) since the struct included blocksync and p2p

@millken
Copy link
Contributor Author

millken commented Jun 9, 2023

image

@@ -42,7 +43,7 @@ type (
// BlockSync defines the interface of blocksyncer
BlockSync interface {
lifecycle.StartStopper

nodestats.StatsReporter
Copy link
Member

Choose a reason for hiding this comment

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

remove this, don't need to explicitly tell, so we can remove the import at L26 above

@@ -98,6 +99,7 @@ type (
// Agent is the agent to help the blockchain node connect into the P2P networks and send/receive messages
Agent interface {
lifecycle.StartStopper
nodestats.StatsReporter
Copy link
Member

Choose a reason for hiding this comment

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

same here

@sonarcloud
Copy link

sonarcloud bot commented Jun 9, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
3.9% 3.9% Duplication

@millken millken merged commit a048de8 into iotexproject:master Jun 9, 2023
3 of 4 checks passed
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.

Log summary about the node status
4 participants