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

feat(dot/telemetry): Added more telemetry messages in grandpa client #2043

Merged
merged 16 commits into from
Nov 30, 2021
Merged
Show file tree
Hide file tree
Changes from 11 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
44 changes: 44 additions & 0 deletions dot/telemetry/afg_authority_set.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2021 ChainSafe Systems (ON) Corp.
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
// This file is part of gossamer.
//
// The gossamer library is free software: you can redistribute it and/or modify
// it under the terms of the GNU Lesser General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// The gossamer library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with the gossamer library. If not, see <http://www.gnu.org/licenses/>.

package telemetry

// afgAuthoritySetTM is a telemetry message of type `afg.authority_set` which is
// meant to be sent when authority set changes (generally when a round is initiated)
type afgAuthoritySetTM struct {
// finalized hash and finalized number are sent by substrate, but not read by
// substrate telemetry
// FinalizedHash *common.Hash `json:"hash"`
// FinalizedNumber string `json:"number"`
Copy link
Contributor

Choose a reason for hiding this comment

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

how much extra effort would it be to add these?

Copy link
Contributor Author

@kishansagathiya kishansagathiya Nov 23, 2021

Choose a reason for hiding this comment

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

None, hash and number can be achieved by s.head.Hash() and s.head.Number.String().

I did not add them because telemetry server does not read those data. substrate also doesn't send these all the time. There is also another telemetry message through which we are sending these info afg.finalized_blocks_up_to

Would you prefer i send hash and number?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah either just add them, or remove this

AuthorityID string `json:"authority_id"`
AuthoritySetID string `json:"authority_set_id"`
// Substrate creates an array of string of authority IDs. It JSON-serialises
// that array and send that as a string.
Authorities string `json:"authorities"`
}

// NewAfgAuthoritySetTM creates a new afgAuthoritySetTM struct.
func NewAfgAuthoritySetTM(authorityID, authoritySetID string, authorities string) Message {
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
return &afgAuthoritySetTM{
AuthorityID: authorityID,
AuthoritySetID: authoritySetID,
Authorities: authorities,
}
}

func (afgAuthoritySetTM) messageType() string {
return afgAuthoritySetMsg
}
40 changes: 40 additions & 0 deletions dot/telemetry/afg_finalized_blocks_up_to.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2021 ChainSafe Systems (ON) Corp.
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
// This file is part of gossamer.
//
// The gossamer library is free software: you can redistribute it and/or modify
// it under the terms of the GNU Lesser General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// The gossamer library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with the gossamer library. If not, see <http://www.gnu.org/licenses/>.

package telemetry

import (
"github.com/ChainSafe/gossamer/lib/common"
)

// afgFinalizedBlocksUpToTM holds telemetry message of type `afg.finalized_blocks_up_to`,
// which is supposed to be send GRANDPA client finalises new blocks.
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
type afgFinalizedBlocksUpToTM struct {
Hash common.Hash `json:"hash"`
Number string `json:"number"`
}

// NewAfgFinalizedBlocksUpToTM creates a new afgFinalizedBlocksUpToTM struct.
func NewAfgFinalizedBlocksUpToTM(hash common.Hash, number string) Message {
qdm12 marked this conversation as resolved.
Show resolved Hide resolved
return &afgFinalizedBlocksUpToTM{
Hash: hash,
Number: number,
}
}

func (afgFinalizedBlocksUpToTM) messageType() string {
return afgFinalizedBlocksUpToMsg
}
2 changes: 1 addition & 1 deletion dot/telemetry/notify_finalized.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type notifyFinalizedTM struct {
Height string `json:"height"`
}

// NewNotifyFinalizedTM gets a new NotifyFinalizedTM struct.
// NewNotifyFinalizedTM gets a new notifyFinalizedTM struct.
func NewNotifyFinalizedTM(best common.Hash, height string) Message {
return &notifyFinalizedTM{
Best: best,
Expand Down
8 changes: 5 additions & 3 deletions dot/telemetry/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ import (

// telemetry message types
const (
notifyFinalizedMsg = "notify.finalized"
blockImportMsg = "block.import"
systemNetworkStateMsg = "system.network_state"
systemConnectedMsg = "system.connected"
systemIntervalMsg = "system.interval"
systemNetworkStateMsg = "system.network_state"
blockImportMsg = "block.import"
notifyFinalizedMsg = "notify.finalized"
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
afgFinalizedBlocksUpToMsg = "afg.finalized_blocks_up_to"
afgAuthoritySetMsg = "afg.authority_set"
txPoolImportMsg = "txpool.import"
preparedBlockForProposingMsg = "prepared_block_for_proposing"
)
Expand Down
9 changes: 7 additions & 2 deletions dot/telemetry/telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,14 @@ func TestMain(m *testing.M) {
func TestHandler_SendMulti(t *testing.T) {
expected := [][]byte{
[]byte(`{"authority":false,"chain":"chain","genesis_hash":"0x91b171bb158e2d3848fa23a9f1c25182fb8e20313b2c1eb49219da7a70ce90c3","implementation":"systemName","msg":"system.connected","name":"nodeName","network_id":"netID","startup_time":"startTime","ts":`), //nolint:lll
[]byte(`{"best":"0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6","height":2,"msg":"block.import","origin":"NetworkInitialSync","ts":`), //nolint:lll
[]byte(`{"best":"0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6","height":2,"msg":"block.import","origin":"NetworkInitialSync","ts":`), //nolint //nolint:lll
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
[]byte(`{"bandwidth_download":2,"bandwidth_upload":3,"msg":"system.interval","peers":1,"ts":`),
[]byte(`{"best":"0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6","finalized_hash":"0x687197c11b4cf95374159843e7f46fbcd63558db981aaef01a8bac2a44a1d6b2","finalized_height":32256,"height":32375,"msg":"system.interval","ts":`), //nolint:lll
[]byte(`{"best":"0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6","height":"32375","msg":"notify.finalized","ts":`), //nolint:lll
[]byte(`{"hash":"0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c","msg":"prepared_block_for_proposing","number":"1","ts":`), //nolint:lll
[]byte(`{"hash":"0x5814aec3e28527f81f65841e034872f3a30337cf6c33b2d258bba6071e37e27c","msg":"prepared_block_for_proposing","number":"1","ts":`), //nolint //nolint:lll
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
[]byte(`{"future":2,"msg":"txpool.import","ready":1,"ts":`),
[]byte(`{"authorities":"json-stringified-ids-of-authorities","authority_id":"authority_id","authority_set_id":"authority_set_id","msg":"afg.authority_set","ts`), //nolint
[]byte(`{"hash":"0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6","msg":"afg.finalized_blocks_up_to","number":"1","ts":`), //nolint
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
}

messages := []Message{
Expand All @@ -76,6 +78,9 @@ func TestHandler_SendMulti(t *testing.T) {
common.MustHexToHash("0x687197c11b4cf95374159843e7f46fbcd63558db981aaef01a8bac2a44a1d6b2"),
),

NewAfgAuthoritySetTM("authority_id", "authority_set_id", "json-stringified-ids-of-authorities"),
NewAfgFinalizedBlocksUpToTM(
common.MustHexToHash("0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6"), "1"),
NewNotifyFinalizedTM(
common.MustHexToHash("0x07b749b6e20fd5f1159153a2e790235018621dd06072a62bcd25e8576f6ff5e6"),
"32375"),
Expand Down
39 changes: 39 additions & 0 deletions lib/grandpa/grandpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ package grandpa
import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"math/big"
"sync"
"sync/atomic"
"time"

"github.com/ChainSafe/gossamer/dot/telemetry"
"github.com/ChainSafe/gossamer/dot/types"
"github.com/ChainSafe/gossamer/internal/log"
"github.com/ChainSafe/gossamer/lib/blocktree"
Expand Down Expand Up @@ -258,13 +260,41 @@ func (s *Service) updateAuthorities() error {
// setting to 0 before incrementing indicates
// the setID has been increased
s.state.round = 0

s.sendTelemetryAuthoritySet()

return nil
}

func (s *Service) publicKeyBytes() ed25519.PublicKeyBytes {
return s.keypair.Public().(*ed25519.PublicKey).AsBytes()
}

func (s *Service) sendTelemetryAuthoritySet() {
authorityID := s.keypair.Public().Hex()
authorities := make([]string, len(s.state.voters))
for i, voter := range s.state.voters {
authorities[i] = fmt.Sprintf("%d", voter.ID)
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
}

authoritiesBytes, err := json.Marshal(authorities)
if err != nil {
logger.Debugf("could not marshal authorities: %s", err)
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
return
}

err = telemetry.GetInstance().SendMessage(
telemetry.NewAfgAuthoritySetTM(
authorityID,
fmt.Sprintf("%d", s.state.setID),
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
string(authoritiesBytes),
),
)
if err != nil {
logger.Debugf("problem sending afg.authority_set telemetry message: %s", err)
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
}
}

func (s *Service) initiateRound() error {
// if there is an authority change, execute it
err := s.updateAuthorities()
Expand Down Expand Up @@ -607,6 +637,15 @@ func (s *Service) attemptToFinalize() error {

logger.Debugf("sending CommitMessage: %v", cm)
s.network.GossipMessage(msg)

err = telemetry.GetInstance().SendMessage(telemetry.NewAfgFinalizedBlocksUpToTM(
s.head.Hash(),
s.head.Number.String(),
))
if err != nil {
logger.Debugf("problem sending `afg.finalized_blocks_up_to` telemetry message: %s", err)
}

return nil
}
}
Expand Down