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

fix(dot/telemetry): telemetry hashes to be in the hexadecimal format #2194

Merged
merged 6 commits into from
Jan 17, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion dot/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,9 @@ func TestInitNode_LoadGenesisData(t *testing.T) {
ctrl := gomock.NewController(t)
telemetryMock := NewMockClient(ctrl)

bestHash := common.MustHexToHash("0x336743aadf42654d4ef91294b61a167c9ed8a42f7f327d08d1e3c99541047392")
expectedArg := &telemetry.NotifyFinalized{
Best: common.MustHexToHash("0x336743aadf42654d4ef91294b61a167c9ed8a42f7f327d08d1e3c99541047392"),
Best: &bestHash,
Height: "0",
}

Expand Down
2 changes: 1 addition & 1 deletion dot/state/block_finalisation.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func (bs *BlockState) SetFinalisedHash(hash common.Hash, round, setID uint64) er

bs.telemetry.SendMessage(
telemetry.NewNotifyFinalized(
header.Hash(),
&hash,
header.Number.String(),
),
)
Expand Down
3 changes: 2 additions & 1 deletion dot/state/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,12 @@ func TestGetStorageChildAndGetStorageFromChild(t *testing.T) {
require.NoError(t, err)

_, genTrie, genHeader := genesis.NewTestGenesisWithTrieAndHeader(t)
genHash := genHeader.Hash()

ctrl := gomock.NewController(t)
telemetryMock := NewMockClient(ctrl)
telemetryMock.EXPECT().SendMessage(telemetry.NewNotifyFinalized(
genHeader.Hash(),
&genHash,
"0",
))

Expand Down
6 changes: 3 additions & 3 deletions dot/telemetry/afg_finalized_blocks_up_to.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ var _ Message = (*AfgFinalizedBlocksUpTo)(nil)
// AfgFinalizedBlocksUpTo holds telemetry message of type `afg.finalized_blocks_up_to`,
// which is supposed to be sent when GRANDPA client finalises new blocks.
type AfgFinalizedBlocksUpTo struct {
Hash common.Hash `json:"hash"`
Number string `json:"number"`
Hash *common.Hash `json:"hash"`
Number string `json:"number"`
}

// NewAfgFinalizedBlocksUpTo creates a new AfgFinalizedBlocksUpToTM struct.
func NewAfgFinalizedBlocksUpTo(hash common.Hash, number string) *AfgFinalizedBlocksUpTo {
func NewAfgFinalizedBlocksUpTo(hash *common.Hash, number string) *AfgFinalizedBlocksUpTo {
return &AfgFinalizedBlocksUpTo{
Hash: hash,
Number: number,
Expand Down
18 changes: 9 additions & 9 deletions dot/telemetry/afg_received.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ var (
)

type afgReceived struct {
TargetHash common.Hash `json:"target_hash"`
Copy link
Contributor

@kishansagathiya kishansagathiya Jan 14, 2022

Choose a reason for hiding this comment

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

can you confirm that substrate-telemetry is able to recognize these messages (afgReceived, AfgFinalizedBlocksUpTo, basically any message type that you changed)

Copy link
Member Author

Choose a reason for hiding this comment

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

@kishansagathiya I've set up 3 gossamer nodes and the substrate-telemetry server and here is the list of messages logged

message type Payload::SystemConnected
message type Payload::SystemInterval
message type Payload::PreparedBlockForProposing
message type Payload::NotifyFinalized
message type Payload::AfgFinalizedBlocksUpTo
message Payload::AfgReceivedPrevote
message Payload::BlockImport
message Payload::AfgReceivedPrecommit
message Payload::AfgReceivedCommit

TargetNumber string `json:"target_number"`
Voter string `json:"voter"`
TargetHash *common.Hash `json:"target_hash"`
TargetNumber string `json:"target_number"`
Voter string `json:"voter"`
}

// AfgReceivedPrecommit holds `afg.received_precommit` telemetry message which is
// supposed to be sent when grandpa client receives a precommit.
type AfgReceivedPrecommit afgReceived

// NewAfgReceivedPrecommit gets a new AfgReceivedPrecommitTM struct.
func NewAfgReceivedPrecommit(targetHash common.Hash, targetNumber, voter string) *AfgReceivedPrecommit {
func NewAfgReceivedPrecommit(targetHash *common.Hash, targetNumber, voter string) *AfgReceivedPrecommit {
return &AfgReceivedPrecommit{
TargetHash: targetHash,
TargetNumber: targetNumber,
Expand All @@ -56,7 +56,7 @@ func (afg AfgReceivedPrecommit) MarshalJSON() ([]byte, error) {
type AfgReceivedPrevote afgReceived

// NewAfgReceivedPrevote gets a new AfgReceivedPrevote* struct.
func NewAfgReceivedPrevote(targetHash common.Hash, targetNumber, voter string) *AfgReceivedPrevote {
func NewAfgReceivedPrevote(targetHash *common.Hash, targetNumber, voter string) *AfgReceivedPrevote {
return &AfgReceivedPrevote{
TargetHash: targetHash,
TargetNumber: targetNumber,
Expand All @@ -83,13 +83,13 @@ type afgReceivedCommitTM AfgReceivedCommit
// AfgReceivedCommit holds `afg.received_commit` telemetry message which is
// supposed to be sent when grandpa client receives a commit.
type AfgReceivedCommit struct {
TargetHash common.Hash `json:"target_hash"`
TargetNumber string `json:"target_number"`
ContainsPrecommitsSignedBy []string `json:"contains_precommits_signed_by"`
TargetHash *common.Hash `json:"target_hash"`
TargetNumber string `json:"target_number"`
ContainsPrecommitsSignedBy []string `json:"contains_precommits_signed_by"`
}

// NewAfgReceivedCommit gets a new AfgReceivedCommit* struct.
func NewAfgReceivedCommit(targetHash common.Hash, targetNumber string,
func NewAfgReceivedCommit(targetHash *common.Hash, targetNumber string,
containsPrecommitsSignedBy []string) *AfgReceivedCommit {
return &AfgReceivedCommit{
TargetHash: targetHash,
Expand Down
80 changes: 43 additions & 37 deletions dot/telemetry/mailer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ func TestHandler_SendMulti(t *testing.T) {
[]byte(`{"best":"0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6","height":2,"origin":"NetworkInitialSync","msg":"block.import","ts":`), //nolint:lll
[]byte(`{"bandwidth_download":2,"bandwidth_upload":3,"peers":1,"msg":"system.interval","ts":`),
[]byte(`{"best":"0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6","height":32375,"finalized_hash":"0x687197c11b4cf95374159843e7f46fbcd63558db981aaef01a8bac2a44a1d6b2","finalized_height":32256,"txcount":0,"used_state_cache_size":1234,"msg":"system.interval","ts":`), //nolint:lll
[]byte(`{"best":[7,183,73,182,226,15,213,241,21,145,83,162,231,144,35,80,24,98,29,208,96,114,166,43,205,37,232,87,111,111,245,230],"height":"32375","msg":"notify.finalized","ts":`), //nolint:lll
[]byte(`{"hash":[88,20,174,195,226,133,39,248,31,101,132,30,3,72,114,243,163,3,55,207,108,51,178,210,88,187,166,7,30,55,226,124],"number":"1","msg":"prepared_block_for_proposing","ts":`), //nolint:lll
[]byte(`{"best":"0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6","height":"32375","msg":"notify.finalized","ts":`), //nolint:lll
[]byte(`{"hash":"0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c","number":"1","msg":"prepared_block_for_proposing","ts":`), //nolint:lll
[]byte(`{"ready":1,"future":2,"msg":"txpool.import","ts":`),
[]byte(`{"authority_id":"authority_id","authority_set_id":"authority_set_id","authorities":"json-stringified-ids-of-authorities","msg":"afg.authority_set","ts`), //nolint:lll
[]byte(`{"hash":[7,183,73,182,226,15,213,241,21,145,83,162,231,144,35,80,24,98,29,208,96,114,166,43,205,37,232,87,111,111,245,230],"number":"1","msg":"afg.finalized_blocks_up_to","ts":`), //nolint:lll
[]byte(`{"target_hash":[88,20,174,195,226,133,39,248,31,101,132,30,3,72,114,243,163,3,55,207,108,51,178,210,88,187,166,7,30,55,226,124],"target_number":"1","contains_precommits_signed_by":[],"msg":"afg.received_commit","ts":`), //nolint:lll
[]byte(`{"target_hash":[88,20,174,195,226,133,39,248,31,101,132,30,3,72,114,243,163,3,55,207,108,51,178,210,88,187,166,7,30,55,226,124],"target_number":"1","voter":"","msg":"afg.received_precommit","ts":`), //nolint:lll
[]byte(`{"target_hash":[88,20,174,195,226,133,39,248,31,101,132,30,3,72,114,243,163,3,55,207,108,51,178,210,88,187,166,7,30,55,226,124],"target_number":"1","voter":"","msg":"afg.received_prevote","ts":`), //nolint:lll
[]byte(`{"authority_id":"authority_id","authority_set_id":"authority_set_id","authorities":"json-stringified-ids-of-authorities","msg":"afg.authority_set","ts`), //nolint:lll
[]byte(`{"hash":"0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6","number":"1","msg":"afg.finalized_blocks_up_to","ts":`), //nolint:lll
[]byte(`{"target_hash":"0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c","target_number":"1","contains_precommits_signed_by":[],"msg":"afg.received_commit","ts":`), //nolint:lll
[]byte(`{"target_hash":"0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c","target_number":"1","voter":"","msg":"afg.received_precommit","ts":`), //nolint:lll
[]byte(`{"target_hash":"0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c","target_number":"1","voter":"","msg":"afg.received_prevote","ts":`), //nolint:lll
}

messages := []Message{
Expand All @@ -95,23 +95,34 @@ func TestHandler_SendMulti(t *testing.T) {
),

NewAfgAuthoritySet("authority_id", "authority_set_id", "json-stringified-ids-of-authorities"),
NewAfgFinalizedBlocksUpTo(
common.MustHexToHash("0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6"), "1"),
NewAfgReceivedCommit(
common.MustHexToHash("0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c"),

func(hash common.Hash, number string) Message {
return NewAfgFinalizedBlocksUpTo(&hash, number)
}(common.MustHexToHash("0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6"), "1"),

func(targetHash common.Hash, targetNumber string, containsPrecommitsSignedBy []string) Message {
return NewAfgReceivedCommit(&targetHash, targetNumber, containsPrecommitsSignedBy)
}(common.MustHexToHash("0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c"),
"1", []string{}),
NewAfgReceivedPrecommit(
common.MustHexToHash("0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c"),

func(targetHash common.Hash, targetNumber string, voter string) Message {
return NewAfgReceivedPrecommit(&targetHash, targetNumber, voter)
}(common.MustHexToHash("0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c"),
"1", ""),
NewAfgReceivedPrevote(
common.MustHexToHash("0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c"),

func(targetHash common.Hash, targetNumber string, voter string) Message {
return NewAfgReceivedPrevote(&targetHash, targetNumber, voter)
}(common.MustHexToHash("0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c"),
"1", ""),

NewNotifyFinalized(
common.MustHexToHash("0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6"),
func(best common.Hash, height string) Message {
return NewNotifyFinalized(&best, height)
}(common.MustHexToHash("0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6"),
"32375"),
NewPreparedBlockForProposing(
common.MustHexToHash("0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c"),

func(hash common.Hash, number string) Message {
return NewPreparedBlockForProposing(&hash, number)
}(common.MustHexToHash("0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c"),
"1"),
}

Expand Down Expand Up @@ -259,44 +270,41 @@ func TestTelemetryMarshalMessage(t *testing.T) {
},
"AfgFinalizedBlocksUpTo_marshal": {
message: &AfgFinalizedBlocksUpTo{
Hash: common.Hash{},
Hash: &common.Hash{},
Number: "0",
},
expected: regexp.MustCompile(`^{"hash":\[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,` +
`0,0,0,0,0,0,0,0,0,0,0,0,0,0,0\],"number":"0",` +
expected: regexp.MustCompile(`^{"hash":"0x[0]{64}","number":"0",` +
`"msg":"afg.finalized_blocks_up_to","ts":"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:` +
`[0-9]{2}.[0-9]+Z|([+-][0-9]{2}:[0-9]{2})"}$`),
},
"AfgReceivedPrecommit_marshal": {
message: &AfgReceivedPrecommit{
TargetHash: common.Hash{},
TargetHash: &common.Hash{},
TargetNumber: "0",
Voter: "0x0",
},
expected: regexp.MustCompile(`^{"target_hash":\[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,` +
`0,0,0,0,0,0,0,0,0,0,0,0,0,0,0\],"target_number":"0","voter":"0x0",` +
expected: regexp.MustCompile(`^{"target_hash":"0x[0]{64}","target_number":"0","voter":"0x0",` +
`"msg":"afg.received_precommit","ts":"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:` +
`[0-9]{2}.[0-9]+Z|([+-][0-9]{2}:[0-9]{2})"}$`),
},
"AfgReceivedPrevoteTM_marshal": {
message: &AfgReceivedPrevote{
TargetHash: common.Hash{},
TargetHash: &common.Hash{},
TargetNumber: "0",
Voter: "0x0",
},
expected: regexp.MustCompile(`^{"target_hash":\[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,` +
`0,0,0,0,0,0,0,0,0,0,0,0,0,0,0\],"target_number":"0","voter":"0x0",` +
expected: regexp.MustCompile(`^{"target_hash":"0x[0]{64}","target_number":"0","voter":"0x0",` +
`"msg":"afg.received_prevote","ts":"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:` +
`[0-9]{2}.[0-9]+Z|([+-][0-9]{2}:[0-9]{2})"}$`),
},
"AfgReceivedCommit_marshal": {
message: &AfgReceivedCommit{
TargetHash: common.Hash{},
TargetHash: &common.Hash{},
TargetNumber: "0",
ContainsPrecommitsSignedBy: []string{"0x0", "0x1"},
},
expected: regexp.MustCompile(`^{"target_hash":\[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,` +
`0,0,0,0,0,0,0,0,0,0,0,0,0,0,0\],"target_number":"0","contains_precommits_signed_by":\["0x0","0x1"\],` +
expected: regexp.MustCompile(`^{"target_hash":"0x[0]{64}","target_number":"0",` +
`"contains_precommits_signed_by":\["0x0","0x1"\],` +
`"msg":"afg.received_commit","ts":"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:` +
`[0-9]{2}.[0-9]+Z|([+-][0-9]{2}:[0-9]{2})"}$`),
},
Expand All @@ -312,21 +320,19 @@ func TestTelemetryMarshalMessage(t *testing.T) {
},
"NotifyFinalized_marshal": {
message: &NotifyFinalized{
Best: common.Hash{},
Best: &common.Hash{},
Height: "0",
},
expected: regexp.MustCompile(`^{"best":\[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,` +
`0,0,0,0,0,0,0,0,0,0,0,0,0,0,0\],"height":"0",` +
expected: regexp.MustCompile(`^{"best":"0x[0]{64}","height":"0",` +
`"msg":"notify.finalized","ts":"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:` +
`[0-9]{2}.[0-9]+Z|([+-][0-9]{2}:[0-9]{2})"}$`),
},
"PreparedBlockForProposing_marshal": {
message: &PreparedBlockForProposing{
Hash: common.Hash{},
Hash: &common.Hash{},
Number: "0",
},
expected: regexp.MustCompile(`^{"hash":\[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,` +
`0,0,0,0,0,0,0,0,0,0,0,0,0,0,0\],"number":"0",` +
expected: regexp.MustCompile(`^{"hash":"0x[0]{64}","number":"0",` +
`"msg":"prepared_block_for_proposing","ts":"[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:` +
`[0-9]{2}.[0-9]+Z|([+-][0-9]{2}:[0-9]{2})"}$`),
},
Expand Down
4 changes: 2 additions & 2 deletions dot/telemetry/notify_finalized.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ var _ Message = (*NotifyFinalized)(nil)
// NotifyFinalized holds `notify.finalized` telemetry message, which is
// supposed to be send when a new block gets finalised.
type NotifyFinalized struct {
Best common.Hash `json:"best"`
Best *common.Hash `json:"best"`
// Height is same as block.Header.Number
Height string `json:"height"`
}

// NewNotifyFinalized gets a new NotifyFinalizedTM struct.
func NewNotifyFinalized(best common.Hash, height string) *NotifyFinalized {
func NewNotifyFinalized(best *common.Hash, height string) *NotifyFinalized {
return &NotifyFinalized{
Best: best,
Height: height,
Expand Down
4 changes: 2 additions & 2 deletions dot/telemetry/prepared_block_for_proposing.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ var _ Message = (*PreparedBlockForProposing)(nil)
// PreparedBlockForProposing holds a 'prepared_block_for_proposing' telemetry
// message, which is supposed to be sent when a new block is built.
type PreparedBlockForProposing struct {
Hash common.Hash `json:"hash"`
Hash *common.Hash `json:"hash"`
// Height of the chain, Block.Header.Number
Number string `json:"number"`
}

// NewPreparedBlockForProposing gets a new PreparedBlockForProposingTM struct.
func NewPreparedBlockForProposing(hash common.Hash, number string) *PreparedBlockForProposing {
func NewPreparedBlockForProposing(hash *common.Hash, number string) *PreparedBlockForProposing {
return &PreparedBlockForProposing{
Hash: hash,
Number: number,
Expand Down
3 changes: 2 additions & 1 deletion lib/babe/babe.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,10 @@ func (b *Service) handleSlot(epoch, slotNum uint64) error {
"built block with parent hash %s, header %s and body %s",
parent.Hash(), block.Header.String(), block.Body)

blockHash := block.Header.Hash()
b.telemetry.SendMessage(
telemetry.NewPreparedBlockForProposing(
block.Header.Hash(),
&blockHash,
block.Header.Number.String(),
),
)
Expand Down
3 changes: 2 additions & 1 deletion lib/grandpa/grandpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -646,8 +646,9 @@ func (s *Service) attemptToFinalize() error {
logger.Debugf("sending CommitMessage: %v", cm)
s.network.GossipMessage(msg)

hash := s.head.Hash()
s.telemetry.SendMessage(telemetry.NewAfgFinalizedBlocksUpTo(
s.head.Hash(),
&hash,
s.head.Number.String(),
))

Expand Down
2 changes: 1 addition & 1 deletion lib/grandpa/message_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (h *MessageHandler) handleCommitMessage(msg *CommitMessage) error {

h.telemetry.SendMessage(
telemetry.NewAfgReceivedCommit(
msg.Vote.Hash,
&msg.Vote.Hash,
fmt.Sprint(msg.Vote.Number),
containsPrecommitsSignedBy,
),
Expand Down
4 changes: 2 additions & 2 deletions lib/grandpa/vote_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ func (s *Service) receiveVoteMessages(ctx context.Context) {
case prevote, primaryProposal:
s.telemetry.SendMessage(
telemetry.NewAfgReceivedPrevote(
vm.Message.Hash,
&vm.Message.Hash,
fmt.Sprint(vm.Message.Number),
vm.Message.AuthorityID.String(),
),
)
case precommit:
s.telemetry.SendMessage(
telemetry.NewAfgReceivedPrecommit(
vm.Message.Hash,
&vm.Message.Hash,
fmt.Sprint(vm.Message.Number),
vm.Message.AuthorityID.String(),
),
Expand Down