-
Notifications
You must be signed in to change notification settings - Fork 189
Vishal/add operator details #185
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
Changes from 2 commits
4436c42
88a08b8
820fd58
b393853
aed0949
99b7e4b
1514f29
ae11607
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ type Engine struct { | |
pingEnabled bool | ||
pingInterval time.Duration | ||
middleware *libp2p.Middleware | ||
nodeInfo map[flow.Identifier]string // additional details about a node such as operator name | ||
} | ||
|
||
func New( | ||
|
@@ -32,7 +33,9 @@ func New( | |
metrics module.PingMetrics, | ||
pingEnabled bool, | ||
mw *libp2p.Middleware, | ||
nodeInfoFile string, | ||
) (*Engine, error) { | ||
|
||
eng := &Engine{ | ||
unit: engine.NewUnit(), | ||
log: log.With().Str("engine", "ping").Logger(), | ||
|
@@ -44,6 +47,20 @@ func New( | |
middleware: mw, | ||
} | ||
|
||
// if a node info file is provided, it is read and the additional node information is reported as part of the ping metric | ||
if nodeInfoFile != "" { | ||
nodeInfo, err := readJson(nodeInfoFile) | ||
if err != nil { | ||
log.Error().Err(err).Str("node_info_file", nodeInfoFile).Msg("failed to read node info file") | ||
} else { | ||
eng.nodeInfo = nodeInfo | ||
log.Debug().Str("node_info_file", nodeInfoFile).Msg("using node info file") | ||
} | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed we don't initialize |
||
// the node info file is not mandatory and should not stop the Ping engine from running | ||
log.Trace().Msg("no node info file specified") | ||
} | ||
|
||
return eng, nil | ||
} | ||
|
||
|
@@ -85,10 +102,12 @@ func (e *Engine) startPing() { | |
} | ||
} | ||
|
||
// send ping to a given node and report the reachable result to metrics | ||
// pingNode pings the given peer and updates the metrics with the result and the additional node information | ||
func (e *Engine) pingNode(peer *flow.Identity) { | ||
reachable := e.pingAddress(peer.ID()) | ||
e.metrics.NodeReachable(peer, reachable) | ||
id := peer.ID() | ||
reachable := e.pingAddress(id) | ||
info := e.nodeInfo[id] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if the id is not found in the node info map? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. then the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought of putting in something |
||
e.metrics.NodeReachable(peer, info, reachable) | ||
} | ||
|
||
// pingAddress sends a ping request to the given address, and block until either receive | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
package ping | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"io/ioutil" | ||
"os" | ||
|
||
"github.com/onflow/flow-go/model/flow" | ||
) | ||
|
||
func readJson(jsonFileName string) (map[flow.Identifier]string, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are much more specialized json reading functions, lets give them more specific names, maybe |
||
|
||
// read the file | ||
byteValue, err := openAndReadFile(jsonFileName) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// unmarshal json data | ||
result, err := unmarshalJson(byteValue) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to unmarshal %s: %w", jsonFileName, err) | ||
} | ||
return result, nil | ||
} | ||
|
||
func openAndReadFile(fileName string) ([]byte, error) { | ||
//open | ||
jsonFile, err := os.Open(fileName) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to open %s: %w", fileName, err) | ||
} | ||
defer jsonFile.Close() | ||
|
||
// read | ||
byteValue, err := ioutil.ReadAll(jsonFile) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to read %s: %w", fileName, err) | ||
} | ||
return byteValue, nil | ||
} | ||
|
||
func unmarshalJson(jsonData []byte) (map[flow.Identifier]string, error) { | ||
var result map[flow.Identifier]string | ||
err := json.Unmarshal(jsonData, &result) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return result, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
package ping | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"testing" | ||
|
||
"github.com/onflow/flow-go/model/flow" | ||
"github.com/onflow/flow-go/utils/unittest" | ||
|
||
"github.com/stretchr/testify/require" | ||
"github.com/stretchr/testify/suite" | ||
) | ||
|
||
type Suite struct { | ||
suite.Suite | ||
} | ||
|
||
func TestHandler(t *testing.T) { | ||
suite.Run(t, new(Suite)) | ||
} | ||
|
||
func TestReadJson(t *testing.T) { | ||
totalNodes := 10 | ||
ids := unittest.IdentifierListFixture(totalNodes) | ||
testJson := make(map[flow.Identifier]string, totalNodes) | ||
for i, id := range ids { | ||
testJson[id] = fmt.Sprintf("Operator%d", i+1) | ||
} | ||
jsonAsBytes, err := json.Marshal(testJson) | ||
require.NoError(t, err) | ||
content, err := unmarshalJson(jsonAsBytes) | ||
require.NoError(t, err) | ||
require.Len(t, content, totalNodes) | ||
for k, v := range testJson { | ||
require.Contains(t, content, k) | ||
require.Equal(t, content[k], v) | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g.
...
--node-info-file /data/nodeinfo.json
...