Skip to content

Commit

Permalink
telemetry: fix err check (pingcap#23006)
Browse files Browse the repository at this point in the history
  • Loading branch information
Tjianke authored Mar 16, 2021
1 parent 5bafb32 commit 7e067b2
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 15 deletions.
15 changes: 12 additions & 3 deletions telemetry/data_cluster_hardware.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ L:
instance := row.GetString(1)
activeItem, ok := itemsByInstance[instance]
if !ok {
hostHash, port := parseAddressAndHash(instance)
hostHash, port, err := parseAddressAndHash(instance)
if err != nil {
return nil, err
}
activeItem = &clusterHardwareItem{
InstanceType: row.GetString(0),
ListenHostHash: hostHash,
Expand Down Expand Up @@ -127,7 +130,10 @@ L:
// Use plain text only when it is in a list that we know safe.
hashedDeviceName = normalizedDiskName
} else {
hashedDeviceName = hashString(normalizedDiskName)
hashedDeviceName, err = hashString(normalizedDiskName)
if err != nil {
return nil, err
}
}
activeDiskItem, ok := activeItem.Disk[hashedDeviceName]
if !ok {
Expand All @@ -141,7 +147,10 @@ L:
// Use plain text only when it is in a list that we know safe.
path = fieldValue
} else {
path = hashString(fieldValue)
path, err = hashString(fieldValue)
if err != nil {
return nil, err
}
}
activeDiskItem[normalizeFieldName("path")] = path
} else if sortedStringContains(sortedDiskAllowedFieldNames, fieldName) {
Expand Down
10 changes: 8 additions & 2 deletions telemetry/data_cluster_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,14 @@ func getClusterInfo(ctx sessionctx.Context) ([]*clusterInfoItem, error) {
if row.Len() < 7 {
continue
}
listenHostHash, listenPort := parseAddressAndHash(row.GetString(1))
statusHostHash, statusPort := parseAddressAndHash(row.GetString(2))
listenHostHash, listenPort, err := parseAddressAndHash(row.GetString(1))
if err != nil {
return nil, err
}
statusHostHash, statusPort, err := parseAddressAndHash(row.GetString(2))
if err != nil {
return nil, err
}
r = append(r, &clusterInfoItem{
InstanceType: row.GetString(0),
ListenHostHash: listenHostHash,
Expand Down
18 changes: 12 additions & 6 deletions telemetry/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,18 @@ import (
)

// hashString returns the SHA1 checksum in hex of the string.
func hashString(text string) string {
func hashString(text string) (string, error) {
hash := sha1.New()
hash.Write([]byte(text))
_, err := hash.Write([]byte(text))
if err != nil {
return "", err
}
hashed := hash.Sum(nil)
return fmt.Sprintf("%x", hashed)
return fmt.Sprintf("%x", hashed), nil
}

// parseAddressAndHash parses an address in HOST:PORT format, returns the hashed host and the port.
func parseAddressAndHash(address string) (string, string) {
func parseAddressAndHash(address string) (string, string, error) {
var host, port string
if !strings.Contains(address, ":") {
host = address
Expand All @@ -48,8 +51,11 @@ func parseAddressAndHash(address string) (string, string) {
port = lastPart
}
}

return hashString(host), port
res, err := hashString(host)
if err != nil {
return "", "", err
}
return res, port, err
}

// See https://stackoverflow.com/a/58026884
Expand Down
12 changes: 8 additions & 4 deletions telemetry/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ func TestT(t *testing.T) {

func (s *testUtilSuite) TestHashString(c *C) {
c.Parallel()

c.Assert(hashString("127.0.0.1"), Equals, "4b84b15bff6ee5796152495a230e45e3d7e947d9")
actual, err := hashString("127.0.0.1")
c.Assert(err, IsNil)
c.Assert(actual, Equals, "4b84b15bff6ee5796152495a230e45e3d7e947d9")
}

func (s *testUtilSuite) TestParseAddress(c *C) {
Expand All @@ -53,8 +54,11 @@ func (s *testUtilSuite) TestParseAddress(c *C) {
{"[2001:db8:85a3:8d3:1319:8a2e:370:7348]:443", "[2001:db8:85a3:8d3:1319:8a2e:370:7348]", "443"},
}
for _, tt := range cases {
host, port := parseAddressAndHash(tt.src)
c.Assert(host, Equals, hashString(tt.expectedHost))
host, port, err := parseAddressAndHash(tt.src)
c.Assert(err, IsNil)
exp, err := hashString(tt.expectedHost)
c.Assert(err, IsNil)
c.Assert(host, Equals, exp)
c.Assert(port, Equals, tt.expectedPort)
}
}

0 comments on commit 7e067b2

Please sign in to comment.