Threholds do not roundtrip properly during marshaling

[adriansmares]

What happened:

I am trying to dynamically build some thresholds for a particular field. My thresholds look as follows:

	thresholds = []data.Threshold{
		{
			Color: "dark-green",
			Value: data.ConfFloat64(math.Inf(-1)),
		},
		{
			Color: "semi-dark-green",
			Value: -130,
		},
		{
			Color: "green",
			Value: -125,
		},
		{
			Color: "yellow",
			Value: -120,
		},
		{
			Color: "light-orange",
			Value: -115,
		},
		{
			Color: "semi-dark-orange",
			Value: -110,
		},
		{
			Color: "red",
			Value: -105,
		},
		{
			Color: "dark-red",
			Value: -100,
		},
	}

The problem is that if I check the schema in the browser response, I see the that the value for the first threshold is 0, not null or missing.

{
  "thresholds": {
    "mode": "absolute",
    "steps": [
      {
        "value": 0,
        "color": "dark-green"
      },
      {
        "value": -130,
        "color": "semi-dark-green"
      },
      {
        "value": -125,
        "color": "green"
      },
      {
        "value": -120,
        "color": "yellow"
      },
      {
        "value": -115,
        "color": "light-orange"
      },
      {
        "value": -110,
        "color": "semi-dark-orange"
      },
      {
        "value": -105,
        "color": "red"
      },
      {
        "value": -100,
        "color": "dark-red"
      }
    ]
  }
}

What you expected to happen:

I expected the first threshold to be rendered as null, but it is rendered as a zero value.

How to reproduce it (as minimally and precisely as possible):

Try to dynamically build thresholds in the backend, for any field.

Anything else we need to know?:

I think this happens because there is another marshaling round trip happening between the plugin and Grafana itself.

The definition of a threshold is as follows:

grafana-plugin-sdk-go/data/field_config.go

Lines 209 to 214 in 3e2ff58

	// Threshold a single step on the threshold list
	type Threshold struct {
	Value ConfFloat64 `json:"value,omitempty"` // First value is always -Infinity serialize to null
	Color string `json:"color,omitempty"`
	State string `json:"state,omitempty"`
	}

Notice that the value is not nullable - even if a null would be present in JSON (from the plugin to Grafana), there would not be a null if you convert to a struct and then back to JSON later - unmarshalling would put a zero value in there. This is I hope trivially visible, but the following test also shows this in Go:

func TestT(t *testing.T) {
	th := []data.Threshold{
		{
			Value: data.ConfFloat64(math.Inf(-1)),
		},
	}

	b, err := json.Marshal(th)
	if err != nil {
		t.Fatal(err)
	}
	if string(b) != `[{"value":null}]` {
		t.Fatal(b)
	}

	th2 := []data.Threshold{}
	if err := json.Unmarshal(b, &th2); err != nil {
		t.Fatal(err)
	}

	if th2[0].Value == 0.0 {
		t.Fatal("first entry not null")
	}
}

The solutions don't look too good:

1. Explicit ConfFloat64 null means that the value should be filled with -Inf or NaN. This is not ideal (null is supposed to be a semantical skip), but I guess it could work ?
2. Make Value nullable. This is again not ideal because currently 0.0 is currently skipped due to omitempty. This means that plugins compiled with older versions of the SDK will turn 0.0 into null during round trip, which is a breaking change.
3. Tell people to use -math.MaxFloat64 for the lower bound instead. It's not really equivalent to having a null I guess, but it's pretty close.

Environment:

• SDK version: v0.161.0
• Grafana version: 9.5.2
• Go version: 1.20.4

[adriansmares]

For the reference, field configuration is JSON encoded in the plugin <-> Grafana communication (L108):

grafana-plugin-sdk-go/data/arrow.go

Lines 87 to 124 in 3e2ff58

	// buildArrowFields builds Arrow field definitions from a Frame.
	func buildArrowFields(f *Frame) ([]arrow.Field, error) {
	arrowFields := make([]arrow.Field, len(f.Fields))
	for i, field := range f.Fields {
	t, nullable, err := fieldToArrow(field)
	if err != nil {
	return nil, err
	}
	tstype, _ := getTypeScriptTypeString(field.Type())
	fieldMeta := map[string]string{
	metadataKeyTSType: tstype,
	}
	if field.Labels != nil {
	if fieldMeta[metadataKeyLabels], err = toJSONString(field.Labels); err != nil {
	return nil, err
	}
	}
	if field.Config != nil {
	str, err := toJSONString(field.Config)
	if err != nil {
	return nil, err
	}
	fieldMeta[metadataKeyConfig] = str
	}
	arrowFields[i] = arrow.Field{
	Name: field.Name,
	Type: t,
	Metadata: arrow.MetadataFrom(fieldMeta),
	Nullable: nullable,
	}
	}
	return arrowFields, nil
	}

As such, unmarshaling has to be done also through JSON, and the null won't replace the default 0 (L340):

grafana-plugin-sdk-go/data/arrow.go

Lines 324 to 352 in 3e2ff58

	func initializeFrameFields(schema *arrow.Schema, frame *Frame) ([]bool, error) {
	nullable := make([]bool, len(schema.Fields()))
	for idx, field := range schema.Fields() {
	sdkField := Field{
	Name: field.Name,
	}
	if labelsAsString, ok := getMDKey(metadataKeyLabels, field.Metadata); ok {
	if err := json.Unmarshal([]byte(labelsAsString), &sdkField.Labels); err != nil {
	return nil, err
	}
	}
	if configAsString, ok := getMDKey(metadataKeyConfig, field.Metadata); ok {
	// make sure that Config is not nil, otherwise create a new one
	if sdkField.Config == nil {
	sdkField.Config = &FieldConfig{}
	}
	if err := json.Unmarshal([]byte(configAsString), sdkField.Config); err != nil {
	return nil, err
	}
	}
	nullable[idx] = field.Nullable
	if err := initializeFrameField(field, idx, nullable, &sdkField); err != nil {
	return nil, err
	}
	frame.Fields = append(frame.Fields, &sdkField)
	}
	return nullable, nil
	}