Skip to content

Commit

Permalink
fixup! fixup! fixup! fixup! fixup! fixup! fixup! fix: fallback to `ga…
Browse files Browse the repository at this point in the history
…uge` for `protofmt`-based negotiations
  • Loading branch information
rexagod committed Jan 14, 2024
1 parent b6496c7 commit e0c3e83
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 28 deletions.
1 change: 1 addition & 0 deletions pkg/customresourcestate/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ func configOverrides(config *Metrics) {
for j := range config.Spec.Resources[i].Metrics {

// Override the metric type to lowercase, so the internals have a single source of truth for metric type definitions.
// This is done as a convenience measure for users, so they don't have to remember the exact casing.
config.Spec.Resources[i].Metrics[j].Each.Type = metric.Type(strings.ToLower(string(config.Spec.Resources[i].Metrics[j].Each.Type)))
}
}
Expand Down
1 change: 1 addition & 0 deletions pkg/customresourcestate/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ var testData string
func Test_Metrics_deserialization(t *testing.T) {
var m Metrics
assert.NoError(t, yaml.NewDecoder(strings.NewReader(testData)).Decode(&m))
configOverrides(&m)
assert.Equal(t, "active_count", m.Spec.Resources[0].Metrics[0].Name)

t.Run("can create resource factory", func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/customresourcestate/registry_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ type compiledMetric interface {

// newCompiledMetric returns a compiledMetric depending on the given metric type.
func newCompiledMetric(m Metric) (compiledMetric, error) {
switch metric.Type(strings.ToLower(string(m.Type))) {
switch m.Type {
case metric.Gauge:
if m.Gauge == nil {
return nil, errors.New("expected each.gauge to not be nil")
Expand Down
5 changes: 0 additions & 5 deletions pkg/metric/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@ var (
// Type represents the type of the metric. See https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metric-types.
type Type string

// String returns the string representation of the metric type.
func (t Type) String() string {
return string(t)
}

// Supported metric types.
var (

Expand Down
2 changes: 1 addition & 1 deletion pkg/metric_generator/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (g *FamilyGenerator) generateHeader() string {
header.WriteString("# TYPE ")
header.WriteString(g.Name)
header.WriteByte(' ')
header.WriteString(g.Type.String())
header.WriteString(string(g.Type))

return header.String()
}
Expand Down
29 changes: 10 additions & 19 deletions pkg/metrics_store/metrics_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,8 @@ func SanitizeHeaders(contentType string, writers MetricsWriterList) MetricsWrite
var lastHeader string
for _, writer := range writers {
if len(writer.stores) > 0 {
for i, header := range writer.stores[0].headers {

// Nothing to sanitize.
if len(header) == 0 {
continue
}
for i := 0; i < len(writer.stores[0].headers); {
header := writer.stores[0].headers[i]

// Removes duplicate headers from the given MetricsWriterList for the same family (generated through CRS).
// These are expected to be consecutive since G** resolution generates groups of similar metrics with same headers before moving onto the next G** spec in the CRS configuration.
Expand All @@ -106,8 +102,8 @@ func SanitizeHeaders(contentType string, writers MetricsWriterList) MetricsWrite

// If the requested content type was proto-based, replace "info" and "statesets" with "gauge", as they are not recognized by Prometheus' protobuf machinery.
if strings.HasPrefix(contentType, expfmt.ProtoType) {
infoTypeString := metric.Info.String()
stateSetTypeString := metric.StateSet.String()
infoTypeString := string(metric.Info)
stateSetTypeString := string(metric.StateSet)
if strings.HasSuffix(header, infoTypeString) {
header = header[:len(header)-len(infoTypeString)] + string(metric.Gauge)
writer.stores[0].headers[i] = header
Expand All @@ -121,22 +117,17 @@ func SanitizeHeaders(contentType string, writers MetricsWriterList) MetricsWrite

// Nullify duplicate headers after the sanitization to not miss out on any new candidates.
if header == lastHeader {
writer.stores[0].headers[i] = ""
writer.stores[0].headers = append(writer.stores[0].headers[:i], writer.stores[0].headers[i+1:]...)

// Do not increment the index, as the next header is now at the current index.
continue
}

// Update the last header.
lastHeader = header
}
}
}

// Remove all empty headers.
for _, writer := range writers {
if len(writer.stores) > 0 {
for i := len(writer.stores[0].headers) - 1; i >= 0; i-- {
if writer.stores[0].headers[i] == "" {
writer.stores[0].headers = append(writer.stores[0].headers[:i], writer.stores[0].headers[i+1:]...)
}
// Move to the next header.
i++
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/metricshandler/metrics_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ func (m *MetricsHandler) Run(ctx context.Context) error {
return ctx.Err()
}

// ServeHTTP implements the http.Handler interface. It writes all generated
// metrics to the response body.
// ServeHTTP implements the http.Handler interface. It writes all generated metrics to the response body.
// Note that all operations defined within this procedure are performed at every request.
func (m *MetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
m.mtx.RLock()
defer m.mtx.RUnlock()
Expand Down

0 comments on commit e0c3e83

Please sign in to comment.