Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
9 changes: 9 additions & 0 deletions .changeset/lucky-pandas-wander.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'grafana-prometheus-datasource': patch
---

Fix: Stop rejecting loosely-typed jsonData. Since promlib v0.0.13 a datasource provisioned through the API, Terraform or an operator with an off-spec value — `"true"` for a boolean, `"1000"` for a number, `2.4` for a version string — failed to load, and every query, health check and metric lookup against it errored. Such values are now coerced where the type can be read and ignored where it cannot, each logging a warning. `timeInterval`, `queryTimeout` and `httpMethod` are unchanged and still reject a wrong type.

An ignored value leaves `seriesLimit` unset rather than 0, so it stays distinguishable from a configured limit of zero and readers still apply their own default.

**Breaking (Go API):** the affected `models.PromOptions` fields change from `string`/`bool`/`float64`/`*int64` to named lenient types (`LenientBool`, `LenientString`, `LenientFloat64`, `LenientExemplarTraceIDDestinations`) with the same underlying types and JSON encoding. Literals still assign and compare as before, but passing one to a `string`, `bool` or `float64` parameter now needs an explicit conversion, e.g. `string(opts.CustomQueryParameters)`. `SeriesLimit` becomes `*LenientFloat64`, matching the frontend's `number`. `HTTPMethod`, `TimeInterval` and `QueryTimeout` keep their existing types.
9 changes: 9 additions & 0 deletions .changeset/thin-weeks-play.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'promlib': patch
---

Fix: Stop rejecting loosely-typed jsonData. Since promlib v0.0.13 a datasource provisioned through the API, Terraform or an operator with an off-spec value — `"true"` for a boolean, `"1000"` for a number, `2.4` for a version string — failed to load, and every query, health check and metric lookup against it errored. Such values are now coerced where the type can be read and ignored where it cannot, each logging a warning. `timeInterval`, `queryTimeout` and `httpMethod` are unchanged and still reject a wrong type.

An ignored value leaves `seriesLimit` unset rather than 0, so it stays distinguishable from a configured limit of zero and readers still apply their own default.

**Breaking (Go API):** the affected `models.PromOptions` fields change from `string`/`bool`/`float64`/`*int64` to named lenient types (`LenientBool`, `LenientString`, `LenientFloat64`, `LenientExemplarTraceIDDestinations`) with the same underlying types and JSON encoding. Literals still assign and compare as before, but passing one to a `string`, `bool` or `float64` parameter now needs an explicit conversion, e.g. `string(opts.CustomQueryParameters)`. `SeriesLimit` becomes `*LenientFloat64`, matching the frontend's `number`. `HTTPMethod`, `TimeInterval` and `QueryTimeout` keep their existing types.
8 changes: 4 additions & 4 deletions pkg/promlib/middleware/custom_query_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ func CustomQueryParameters(logger log.Logger, jsonData *models.PromOptions) sdkh
return next
}

customQueryParams := jsonData.CustomQueryParameters
warnVal := jsonData.MaxSamplesProcessedWarningThreshold
errVal := jsonData.MaxSamplesProcessedErrorThreshold
queryStatsEnabled := jsonData.QueryStatsEnabled
customQueryParams := string(jsonData.CustomQueryParameters)
warnVal := float64(jsonData.MaxSamplesProcessedWarningThreshold)
errVal := float64(jsonData.MaxSamplesProcessedErrorThreshold)
queryStatsEnabled := bool(jsonData.QueryStatsEnabled)

if customQueryParams == "" && warnVal == 0 && errVal == 0 && !queryStatsEnabled {
return next
Expand Down
166 changes: 166 additions & 0 deletions pkg/promlib/models/lenient.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
package models

import (
"encoding/json"
"strconv"
"strings"

"github.com/grafana/grafana-plugin-sdk-go/backend/log"
)

// maxLoggedValueLen keeps a stored object or array from filling a log line.
const maxLoggedValueLen = 64

func coerced(toValueType, fromValueType string, data []byte) {
logLenient(fromValueType, toValueType, "coerced", data)
}

func dropped(toValueType, fromValueType string, data []byte) {
logLenient(fromValueType, toValueType, "dropped", data)
}

func logLenient(fromValueType, toValueType, outcome string, data []byte) {
value := string(data)
if len(value) > maxLoggedValueLen {
value = value[:maxLoggedValueLen] + "…"
}
log.DefaultLogger.Warn("datasource jsonData value does not match its declared type",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you pipe in the prometheus logger here so it has a little more context?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

is there anything specific from the additional context that you think would be helpful to add here? piping the prom logger has some trade-offs. Because these unmarshalling functions are called directly from encoding/json looking would have to live outside of the UnmarshalJSON functions and moved into ParsePromOptions, it requires additional parsing and we'd lose information about which path the specific value took (i.e. from int64 to string).

Would love to hear your thoughts on this

"from", fromValueType, "to", toValueType, "outcome", outcome, "value", value)
}

// LenientBool also accepts the string and numeric spellings of a boolean.
type LenientBool bool

func (b *LenientBool) UnmarshalJSON(data []byte) error {
var value bool
if err := json.Unmarshal(data, &value); err == nil {
*b = LenientBool(value)
return nil
}

var str string
if err := json.Unmarshal(data, &str); err == nil {
if parsed, err := strconv.ParseBool(strings.TrimSpace(str)); err == nil {
*b = LenientBool(parsed)
coerced("bool", "string", data)
return nil
}
dropped("bool", "string", data)
return nil
}

var number float64
if err := json.Unmarshal(data, &number); err == nil {
*b = LenientBool(number != 0)
coerced("bool", "float64", data)
return nil
}

dropped("bool", "unknown", data)
return nil
}

// LenientString also accepts a scalar, keeping its JSON text, so an identifier or version
// that YAML turned into a number (prometheusVersion: 2.4) is not blanked.
type LenientString string

func (s *LenientString) UnmarshalJSON(data []byte) error {
var str string
if err := json.Unmarshal(data, &str); err == nil {
*s = LenientString(str)
return nil
}

var number float64
if err := json.Unmarshal(data, &number); err == nil {
*s = LenientString(strings.TrimSpace(string(data)))
coerced("string", "float64", data)
return nil
}

var boolean bool
if err := json.Unmarshal(data, &boolean); err == nil {
*s = LenientString(strings.TrimSpace(string(data)))
coerced("string", "bool", data)
return nil
}

dropped("string", "unknown", data)
return nil
}

// LenientFloat64 also accepts a quoted number.
type LenientFloat64 float64

func (f *LenientFloat64) UnmarshalJSON(data []byte) error {
value, from, ok := readFloat64(data)
if !ok {
dropped("float64", from, data)
return nil
}

*f = LenientFloat64(value)
if from != "" {
coerced("float64", from, data)
}

return nil
}

// readFloat64 reports what LenientFloat64 reads, which JSON type it came from ("" meaning the
// declared type, so no leniency), and whether it could be read at all. Split out so
// clearDroppedPointers can ask the same question without logging and skewing the counts.
func readFloat64(data []byte) (value float64, from string, ok bool) {
var number float64
if err := json.Unmarshal(data, &number); err == nil {
// value was expected float64
return number, "", true
}

var str string
if err := json.Unmarshal(data, &str); err == nil {
if parsed, err := strconv.ParseFloat(strings.TrimSpace(str), 64); err == nil {
// value was a number string
return parsed, "string", true
}
// value was a non number string (i.e. "ten")
return 0, "string", false
}

// value was an unsupported value to coerce from.
return 0, "unknown", false
}

// LenientExemplarTraceIDDestinations ignores a value it cannot read. It does not salvage a
// partial one: a guess would only disagree with what the frontend reads from jsonData.
type LenientExemplarTraceIDDestinations []ExemplarTraceIDDestination

func (d *LenientExemplarTraceIDDestinations) UnmarshalJSON(data []byte) error {
var destinations []ExemplarTraceIDDestination
if err := json.Unmarshal(data, &destinations); err == nil {
*d = destinations
return nil
}

dropped("exemplarDestinations", "unknown", data)
return nil
}

// encoding/json allocates a pointer field before the lenient type sees the value, so a dropped
// value leaves it non-nil at zero — indistinguishable from a stored 0, which for seriesLimit is
// the difference between "apply your own default" and "limit is zero". A lenient type is handed
// a pointer to the allocated value, never to the field, so only the parser can restore nil.
func (o *PromOptions) clearDroppedPointers(data []byte) {
var raw map[string]json.RawMessage
if err := json.Unmarshal(data, &raw); err != nil {
return
}

if value, ok := raw["seriesLimit"]; ok {
if _, _, readable := readFloat64(value); !readable {
// value was an unsupported value to coerce from
// set to nil so ensure it is not confused with a stored 0
o.SeriesLimit = nil
}
}
}
Loading
Loading