Skip to content

Commit

Permalink
Replace ReportComponentStatusIf with ReportComponentOKIfStarting
Browse files Browse the repository at this point in the history
The flexibility of ReportComponentStatusIf invites misuse if the API
is not fully understood. In addition, we only use for the specific case
of conditionally reporting StatusOK if a component's current status is
Starting. This commit replaces ReportComponentStatusIf with
ReportComponentOKIfStarting which fulfills the requirements without
the potential for misuse.
  • Loading branch information
mwear committed Nov 16, 2023
1 parent d1fd02a commit 242ef9a
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 39 deletions.
6 changes: 1 addition & 5 deletions service/extensions/extensions.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,7 @@ func (bes *Extensions) Start(ctx context.Context, host component.Host) error {
)
return err
}
_ = bes.telemetry.Status.ReportComponentStatusIf(
instanceID,
component.NewStatusEvent(component.StatusOK),
func(st component.Status) bool { return st == component.StatusStarting },
)
_ = bes.telemetry.Status.ReportComponentOKIfStarting(instanceID)
extLogger.Info("Extension started.")
}
return nil
Expand Down
6 changes: 1 addition & 5 deletions service/internal/graph/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,11 +399,7 @@ func (g *Graph) StartAll(ctx context.Context, host component.Host) error {
return compErr
}

_ = g.telemetry.Status.ReportComponentStatusIf(
instanceID,
component.NewStatusEvent(component.StatusOK),
func(st component.Status) bool { return st == component.StatusStarting },
)
_ = g.telemetry.Status.ReportComponentOKIfStarting(instanceID)
}
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,5 @@ func TestNewNopSettings(t *testing.T) {
component.NewStatusEvent(component.StatusStarting),
),
)
require.NoError(t,
set.Status.ReportComponentStatusIf(
&component.InstanceID{},
component.NewStatusEvent(component.StatusStarting),
func(component.Status) bool { return true },
),
)
require.NoError(t, set.Status.ReportComponentOKIfStarting(&component.InstanceID{}))
}
8 changes: 1 addition & 7 deletions service/internal/servicetelemetry/telemetry_settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,7 @@ func TestSettings(t *testing.T) {
component.NewStatusEvent(component.StatusStarting),
),
)
require.NoError(t,
set.Status.ReportComponentStatusIf(
&component.InstanceID{},
component.NewStatusEvent(component.StatusStarting),
func(component.Status) bool { return true },
),
)
require.NoError(t, set.Status.ReportComponentOKIfStarting(&component.InstanceID{}))

compSet := set.ToComponentTelemetrySettings(&component.InstanceID{})
require.NoError(t, compSet.ReportComponentStatus(component.NewStatusEvent(component.StatusStarting)))
Expand Down
13 changes: 4 additions & 9 deletions service/internal/status/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,21 +129,16 @@ func (r *Reporter) ReportComponentStatus(
return r.componentFSM(id).transition(ev)
}

// ReportComponentStatusIf reports status for the given InstanceID, if cond, a predicate that
// receives the current status as an argument, evaluates to true.
func (r *Reporter) ReportComponentStatusIf(
id *component.InstanceID,
ev *component.StatusEvent,
cond func(component.Status) bool,
) error {
// ReportComponentOkIfStarting reports StatusOK if the component's current status is Starting
func (r *Reporter) ReportComponentOKIfStarting(id *component.InstanceID) error {
r.mu.Lock()
defer r.mu.Unlock()
if !r.ready {
return errStatusNotReady
}
fsm := r.componentFSM(id)
if cond(fsm.current.Status()) {
return fsm.transition(ev)
if fsm.current.Status() == component.StatusStarting {
return fsm.transition(component.NewStatusEvent(component.StatusOK))
}
return nil
}
Expand Down
8 changes: 2 additions & 6 deletions service/internal/status/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func TestReporterReady(t *testing.T) {
require.NoError(t, err)
}

func TestReportStatusIf(t *testing.T) {
func TestReportComponentOKIfStarting(t *testing.T) {
for _, tc := range []struct {
name string
initialStatuses []component.Status
Expand Down Expand Up @@ -333,11 +333,7 @@ func TestReportStatusIf(t *testing.T) {
require.NoError(t, err)
}

err := rep.ReportComponentStatusIf(
id,
component.NewStatusEvent(component.StatusOK),
func(st component.Status) bool { return st == component.StatusStarting },
)
err := rep.ReportComponentOKIfStarting(id)

require.NoError(t, err)
require.Equal(t, tc.expectedStatuses, receivedStatuses)
Expand Down

0 comments on commit 242ef9a

Please sign in to comment.