healthcheck: only report transitions (#5068)

internal/zero/reconciler/report_status.go

@@ -6,14 +6,11 @@ import (
 	"github.com/pomerium/pomerium/pkg/health"
 )
 
-// sourceAttr is to indicate the source of this health check is not host specific
-var sourceAttr = health.StrAttr("source", "pomerium-managed-core")
-
 func (c *service) ReportBundleAppliedSuccess(
 	bundleID string,
 	metadata map[string]string,
 ) {
-	attr := []health.Attr{sourceAttr}
+	var attr []health.Attr
 	for k, v := range metadata {
 		attr = append(attr, health.StrAttr(fmt.Sprintf("download-metadata-%s", k), v))
 	}
@@ -24,5 +21,5 @@ func (c *service) ReportBundleAppliedFailure(
 	bundleID string,
 	err error,
 ) {
-	health.ReportError(health.ZeroResourceBundle(bundleID), err, sourceAttr)
+	health.ReportError(health.ZeroResourceBundle(bundleID), err)
 }

pkg/health/deduplicate.go

@@ -0,0 +1,80 @@
+package health
+
+import (
+	"sync"
+
+	"golang.org/x/exp/maps"
+)
+
+var _ Provider = (*deduplicator)(nil)
+
+// deduplicator is a health check provider that deduplicates health check reports
+// i.e. it only reports a health check if the status or attributes have changed
+type deduplicator struct {
+	seen     sync.Map
+	provider Provider
+}
+
+type record struct {
+	attr map[string]string
+	err  *string
+}
+
+func newOKRecord(attrs []Attr) *record {
+	return newRecord(nil, attrs)
+}
+
+func newErrorRecord(err error, attrs []Attr) *record {
+	errTxt := err.Error()
+	return newRecord(&errTxt, attrs)
+}
+
+func newRecord(err *string, attrs []Attr) *record {
+	r := &record{err: err, attr: make(map[string]string)}
+	for _, a := range attrs {
+		r.attr[a.Key] = a.Value
+	}
+	return r
+}
+
+func (r *record) Equals(other *record) bool {
+	return r.equalError(other) &&
+		maps.Equal(r.attr, other.attr)
+}
+
+func (r *record) equalError(other *record) bool {
+	if r.err == nil || other.err == nil {
+		return r.err == other.err
+	}
+	return *r.err == *other.err
+}
+
+func NewDeduplicator(provider Provider) Provider {
+	return &deduplicator{provider: provider}
+}
+
+func (d *deduplicator) swap(check Check, next *record) *record {
+	prev, there := d.seen.Swap(check, next)
+	if !there {
+		return nil
+	}
+	return prev.(*record)
+}
+
+// ReportError implements the Provider interface
+func (d *deduplicator) ReportError(check Check, err error, attrs ...Attr) {
+	cur := newErrorRecord(err, attrs)
+	prev := d.swap(check, cur)
+	if prev == nil || !cur.Equals(prev) {
+		d.provider.ReportError(check, err, attrs...)
+	}
+}
+
+// ReportOK implements the Provider interface
+func (d *deduplicator) ReportOK(check Check, attrs ...Attr) {
+	cur := newOKRecord(attrs)
+	prev := d.swap(check, cur)
+	if prev == nil || !cur.Equals(prev) {
+		d.provider.ReportOK(check, attrs...)
+	}
+}

pkg/health/deduplicate_test.go

@@ -0,0 +1,53 @@
+package health_test
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/golang/mock/gomock"
+
+	health "github.com/pomerium/pomerium/pkg/health"
+)
+
+//go:generate go run github.com/golang/mock/mockgen -package health_test -destination provider_mock_test.go github.com/pomerium/pomerium/pkg/health Provider
+
+func TestDeduplicate(t *testing.T) {
+	t.Parallel()
+
+	p := NewMockProvider(gomock.NewController(t))
+	dp := health.NewDeduplicator(p)
+
+	check1, check2 := health.Check("check-1"), health.Check("check-2")
+	p.EXPECT().ReportOK(check1).Times(1)
+	p.EXPECT().ReportOK(check2).Times(1)
+	dp.ReportOK(check1)
+	dp.ReportOK(check2)
+	dp.ReportOK(check1)
+
+	p.EXPECT().ReportError(check1, gomock.Any()).Times(1)
+	dp.ReportError(check1, errors.New("error"))
+	dp.ReportError(check1, errors.New("error"))
+
+	p.EXPECT().ReportOK(check1).Times(1)
+	dp.ReportOK(check1)
+
+	p.EXPECT().ReportOK(check1, health.StrAttr("k1", "v1")).Times(2)
+	p.EXPECT().ReportOK(check1, health.StrAttr("k1", "v2")).Times(1)
+	dp.ReportOK(check1, health.StrAttr("k1", "v1"))
+	dp.ReportOK(check1, health.StrAttr("k1", "v2"))
+	dp.ReportOK(check1, health.StrAttr("k1", "v1"))
+}
+
+func TestDefault(t *testing.T) {
+	t.Parallel()
+
+	p := NewMockProvider(gomock.NewController(t))
+	health.SetProvider(p)
+
+	check1 := health.Check("check-1")
+	p.EXPECT().ReportOK(check1).Times(1)
+	health.ReportOK(check1)
+
+	health.SetProvider(nil)
+	health.ReportOK(check1)
+}

pkg/health/provider.go

@@ -39,6 +39,9 @@ type Provider interface {
 
 // SetProvider sets the health check provider
 func SetProvider(p Provider) {
+	if p != nil {
+		p = NewDeduplicator(p)
+	}
 	defaultProvider.Store(p)
 }
 

pkg/health/provider_mock_test.go

@@ -0,0 +1,69 @@
+// Code generated by MockGen. DO NOT EDIT.
+// Source: github.com/pomerium/pomerium/pkg/health (interfaces: Provider)
+
+// Package health_test is a generated GoMock package.
+package health_test
+
+import (
+	reflect "reflect"
+
+	gomock "github.com/golang/mock/gomock"
+	health "github.com/pomerium/pomerium/pkg/health"
+)
+
+// MockProvider is a mock of Provider interface.
+type MockProvider struct {
+	ctrl     *gomock.Controller
+	recorder *MockProviderMockRecorder
+}
+
+// MockProviderMockRecorder is the mock recorder for MockProvider.
+type MockProviderMockRecorder struct {
+	mock *MockProvider
+}
+
+// NewMockProvider creates a new mock instance.
+func NewMockProvider(ctrl *gomock.Controller) *MockProvider {
+	mock := &MockProvider{ctrl: ctrl}
+	mock.recorder = &MockProviderMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockProvider) EXPECT() *MockProviderMockRecorder {
+	return m.recorder
+}
+
+// ReportError mocks base method.
+func (m *MockProvider) ReportError(arg0 health.Check, arg1 error, arg2 ...health.Attr) {
+	m.ctrl.T.Helper()
+	varargs := []interface{}{arg0, arg1}
+	for _, a := range arg2 {
+		varargs = append(varargs, a)
+	}
+	m.ctrl.Call(m, "ReportError", varargs...)
+}
+
+// ReportError indicates an expected call of ReportError.
+func (mr *MockProviderMockRecorder) ReportError(arg0, arg1 interface{}, arg2 ...interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	varargs := append([]interface{}{arg0, arg1}, arg2...)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReportError", reflect.TypeOf((*MockProvider)(nil).ReportError), varargs...)
+}
+
+// ReportOK mocks base method.
+func (m *MockProvider) ReportOK(arg0 health.Check, arg1 ...health.Attr) {
+	m.ctrl.T.Helper()
+	varargs := []interface{}{arg0}
+	for _, a := range arg1 {
+		varargs = append(varargs, a)
+	}
+	m.ctrl.Call(m, "ReportOK", varargs...)
+}
+
+// ReportOK indicates an expected call of ReportOK.
+func (mr *MockProviderMockRecorder) ReportOK(arg0 interface{}, arg1 ...interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	varargs := append([]interface{}{arg0}, arg1...)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReportOK", reflect.TypeOf((*MockProvider)(nil).ReportOK), varargs...)
+}