Skip to content

Commit

Permalink
[#1629] fix(operator): Support parsing NaN float value in metrics (#1630
Browse files Browse the repository at this point in the history
)

### What changes were proposed in this pull request?
When parsing json, handle special cases where the value might be NaN

### Why are the changes needed?
Fix: #1629 

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
test in real cluster and add unit test
  • Loading branch information
zhengchenyu authored Apr 16, 2024
1 parent 91191e6 commit b4c92b8
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 4 deletions.
37 changes: 33 additions & 4 deletions deploy/kubernetes/operator/pkg/webhook/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"io"
"io/ioutil"
"math"
"net/http"
"time"

Expand Down Expand Up @@ -158,12 +159,40 @@ func NeedInspectPod(pod *corev1.Pod) bool {
return false
}

// JSONFloat is used to parse the float64 which may be NaN
type JSONFloat float64

// MarshalJSON return bytes representing JSONFloat
func (j JSONFloat) MarshalJSON() ([]byte, error) {
v := float64(j)
if math.IsNaN(v) {
s := "\"NaN\""
return []byte(s), nil
}
return json.Marshal(v) // marshal result as standard float64
}

// UnmarshalJSON return the parsed JSONFloat
func (j *JSONFloat) UnmarshalJSON(v []byte) error {
if s := string(v); s == "\"NaN\"" {
*j = JSONFloat(math.NaN())
return nil
}
// just a regular float value
var fv float64
if err := json.Unmarshal(v, &fv); err != nil {
return err
}
*j = JSONFloat(fv)
return nil
}

// MetricItem records an item of metric information of shuffle servers.
type MetricItem struct {
Name string `json:"name"`
LabelNames []string `json:"labelNames"`
LabelValues []string `json:"labelValues"`
Value float32 `json:"value"`
Name string `json:"name"`
LabelNames []string `json:"labelNames"`
LabelValues []string `json:"labelValues"`
Value JSONFloat `json:"value"`
}

// MetricList records all items of metric information of shuffle servers.
Expand Down
72 changes: 72 additions & 0 deletions deploy/kubernetes/operator/pkg/webhook/util/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package util

import (
"encoding/json"
"math"
"testing"
)

// JSONFloatWrapper is struct which contain JSONFloat
type JSONFloatWrapper struct {
Value1 JSONFloat `json:"value1"`
Value2 JSONFloat `json:"value2"`
}

func TestParseJsonFloat(t *testing.T) {
wrapper := JSONFloatWrapper{Value1: JSONFloat(math.NaN()), Value2: 9.99}
bytes, err := json.Marshal(wrapper)
if err != nil {
t.Fatal("Marshal JsonFloat failed, caused by ", err.Error())
}
parsed := &JSONFloatWrapper{}
if err := json.Unmarshal(bytes, parsed); err != nil {
t.Fatal("Unmarshal JsonFloat failed, caused by ", err.Error())
} else if !math.IsNaN(float64(parsed.Value1)) {
t.Fatal("Value1 should be Nan")
} else if math.Abs(float64(parsed.Value2)-9.99) > 1e-9 {
t.Fatal("Value1 should be 9.99")
}
}

func TestGetLastAppNum(t *testing.T) {
jsonString := `
{
"metrics": [{
"name": "app_num_with_node",
"labelNames": ["tags"],
"labelValues": ["ss_v5,GRPC"],
"value": 10.0,
"timestampMs": null
}, {
"name": "total_remove_resource_time",
"labelNames": ["quantile"],
"labelValues": ["0.5"],
"value": "NaN",
"timestampMs": null
}],
"timeStamp": 1712575271639
}
`
if num, err := getLastAppNum([]byte(jsonString)); err != nil {
t.Fatal("getLastAppNum failed, casued by ", err.Error())
} else if num != 10 {
t.Fatal("Get wrong app number: ", num)
}
}

0 comments on commit b4c92b8

Please sign in to comment.