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.

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

Add metrics label for missing val power #113

Merged
merged 5 commits into from
Apr 7, 2023
Merged

Add metrics label for missing val power #113

merged 5 commits into from
Apr 7, 2023

Conversation

BrandonWeng
Copy link
Contributor

@BrandonWeng BrandonWeng commented Apr 7, 2023 •
edited
Loading

Describe your changes and provide context

/sei-tendermint/internal/consensus/state.go:482 +0x148\n"
2:07PM INF stopping service module=consensus service=baseWAL wal=/Users/brandon/.sei/data/cs.wal/wal
2:07PM INF stopping service module=consensus service=Group wal=/Users/brandon/.sei/data/cs.wal/wal
panic: inconsistent label cardinality: expected 1 label values but got 2 in prometheus.Labels{"chain_id":"sei-chain", "validator_address":"96ACAECB1CCF03E2228D035D92F368D242D57A00"} [recovered]
        panic: inconsistent label cardinality: expected 1 label values but got 2 in prometheus.Labels{"chain_id":"sei-chain", "validator_address":"96ACAECB1CCF03E2228D035D92F368D242D57A00"} [recovered]
        panic: inconsistent label cardinality: expected 1 label values but got 2 in prometheus.Labels{"chain_id":"sei-chain", "validator_address":"96ACAECB1CCF03E2228D035D92F368D242D57A00"} [recovered]
        panic: inconsistent label cardinality: expected 1 label values but got 2 in prometheus.Labels{"chain_id":"sei-chain", "validator_address":"96ACAECB1CCF03E2228D035D92F368D242D57A00"} [recovered]
        panic: inconsistent label cardinality: expected 1 label values but got 2 in prometheus.Labels{"chain_id":"sei-chain", "validator_address":"96ACAECB1CCF03E2228D035D92F368D242D57A00"}

goroutine 358 [running]:
github.com/tendermint/tendermint/internal/consensus.(*State).receiveRoutine.func2()
        /Users/brandon/sei/sei-tendermint/internal/consensus/state.go:967 +0x1c4
panic({0x102469ee0, 0x14000885a00})
        /opt/homebrew/opt/go/libexec/src/runtime/panic.go:884 +0x204
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End.func1()
        /Users/brandon/go/pkg/mod/go.opentelemetry.io/otel/[email protected]/trace/span.go:363 +0x2c
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End(0x140044cf080, {0x0, 0x0, 0x1?})
        /Users/brandon/go/pkg/mod/go.opentelemetry.io/otel/[email protected]/trace/span.go:402 +0x6e8
panic({0x102469ee0, 0x14000885a00})
        /opt/homebrew/opt/go/libexec/src/runtime/panic.go:884 +0x204
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End.func1()
        /Users/brandon/go/pkg/mod/go.opentelemetry.io/otel/[email protected]/trace/span.go:363 +0x2c
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End(0x140044cf200, {0x0, 0x0, 0x1?})
        /Users/brandon/go/pkg/mod/go.opentelemetry.io/otel/[email protected]/trace/span.go:402 +0x6e8
panic({0x102469ee0, 0x14000885a00})
        /opt/homebrew/opt/go/libexec/src/runtime/panic.go:884 +0x204
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End.func1()
        /Users/brandon/go/pkg/mod/go.opentelemetry.io/otel/[email protected]/trace/span.go:363 +0x2c
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End(0x140044cf380, {0x0, 0x0, 0xd34?})

Testing performed to validate your change

Able to start up a local chain after the fix

BrandonWeng
BrandonWeng commented Apr 7, 2023
View reviewed changes
internal/consensus/metrics.go
@@ -44,7 +44,7 @@ type Metrics struct {
// Number of validators who did not sign.
MissingValidators metrics.Gauge
// Total power of the missing validators.
MissingValidatorsPower metrics.Gauge
MissingValidatorsPower metrics.Gauge `metrics_labels:"validator_address"`
Copy link
Contributor Author

@BrandonWeng BrandonWeng Apr 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This fails right now since it's not expecting any labels

sei-tendermint/internal/consensus/state.go

Lines 2254 to 2258 in 1e67899

missingValidatorsPower += val.VotingPower
cs.metrics.MissingValidatorsPower.With("validator_address", val.Address.String()).Set(float64(val.VotingPower))
} else {
cs.metrics.MissingValidatorsPower.With("validator_address", val.Address.String()).Set(0)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

nice catch

yzang2019
yzang2019 approved these changes Apr 7, 2023
View reviewed changes
internal/consensus/metrics.go
@@ -44,7 +44,7 @@ type Metrics struct {
// Number of validators who did not sign.
MissingValidators metrics.Gauge
// Total power of the missing validators.
MissingValidatorsPower metrics.Gauge
MissingValidatorsPower metrics.Gauge `metrics_labels:"validator_address"`
Copy link
Contributor

Choose a reason for hiding this comment

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

nice catch

@BrandonWeng BrandonWeng merged commit 7c2f07e into main Apr 7, 2023
@BrandonWeng BrandonWeng deleted the bweng-fix-val branch April 7, 2023 18:11
Timwood0x10 pushed a commit to Timwood0x10/sei-tendermint that referenced this pull request Jun 7, 2023
@BrandonWeng
CacheKv optimizations (sei-protocol#113)
a7c6045 
## Describe your changes and provide context
Cherry picking changes here:
* [\#14168](cosmos/cosmos-sdk#14168) perf:
store/cachekv: preallocate kvL in dirtyItems which gets appended too
* [\#10024](cosmos/cosmos-sdk#10024) fix!:
store/cachekv: reduce growth factor for iterator ranging using binary
searches #10024

## Testing performed to validate your change
Ran a cluster

![image](https://user-images.githubusercontent.com/18161326/207636380-1c404827-cdb4-4bdc-aade-bfc0294f6cd4.png)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yzang2019 yzang2019 yzang2019 approved these changes

@LCyson LCyson LCyson approved these changes

@philipsu522 philipsu522 philipsu522 approved these changes

Assignees

@philipsu522 philipsu522

@yzang2019 yzang2019

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@BrandonWeng @philipsu522 @LCyson @yzang2019