From 1415fc5014e185edb0eac6d502cc2782028b97b0 Mon Sep 17 00:00:00 2001 From: Matthew Wear Date: Thu, 9 Nov 2023 10:24:22 -0800 Subject: [PATCH] Cleanup --- .chloggen/automated-status-on-start.yaml | 4 ++-- component/telemetry.go | 4 ++-- config/confighttp/confighttp_test.go | 20 ++++++++++++++++--- service/extensions/extensions.go | 15 +++++++++++--- service/internal/graph/graph.go | 15 +++++++++++--- .../servicetelemetry/telemetry_settings.go | 6 +++--- service/internal/status/status.go | 3 ++- 7 files changed, 50 insertions(+), 17 deletions(-) diff --git a/.chloggen/automated-status-on-start.yaml b/.chloggen/automated-status-on-start.yaml index 8650de066f3c..8eb1df926b61 100755 --- a/.chloggen/automated-status-on-start.yaml +++ b/.chloggen/automated-status-on-start.yaml @@ -7,10 +7,10 @@ change_type: enhancement component: statusreporting # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: Automates status reporting when `Start` returns. +note: Automates status reporting upon the completion of component.Start(). # One or more tracking issues or pull requests related to the change -issues: [] +issues: [7682] # (Optional) One or more lines of additional information to render under the primary note. # These lines will be padded with 2 spaces and then inserted directly into the document. diff --git a/component/telemetry.go b/component/telemetry.go index 7938495798c7..9602a7a17584 100644 --- a/component/telemetry.go +++ b/component/telemetry.go @@ -31,8 +31,8 @@ type TelemetrySettingsBase struct { Resource pcommon.Resource } -// TelemetrySettings and servicetelemetry.Settings differ in the method signature for -// ReportComponentStatus +// TelemetrySettings and servicetelemetry.TelemetrySettings differ in their +// mechanism for reporting component status. type TelemetrySettings struct { *TelemetrySettingsBase diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index aa4cbe62c32a..4e71b46b5c7f 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -652,7 +652,12 @@ func TestHttpReception(t *testing.T) { return rt, nil } } - client, errClient := hcs.ToClient(componenttest.NewNopHost(), component.TelemetrySettings{}) + client, errClient := hcs.ToClient( + componenttest.NewNopHost(), + component.TelemetrySettings{ + TelemetrySettingsBase: &component.TelemetrySettingsBase{}, + }, + ) require.NoError(t, errClient) resp, errResp := client.Get(hcs.Endpoint) @@ -1288,12 +1293,21 @@ func BenchmarkHttpRequest(b *testing.B) { b.Run(bb.name, func(b *testing.B) { var c *http.Client if !bb.clientPerThread { - c, err = hcs.ToClient(componenttest.NewNopHost(), component.TelemetrySettings{}) + c, err = hcs.ToClient( + componenttest.NewNopHost(), + component.TelemetrySettings{ + TelemetrySettingsBase: &component.TelemetrySettingsBase{}, + }, + ) require.NoError(b, err) } b.RunParallel(func(pb *testing.PB) { if c == nil { - c, err = hcs.ToClient(componenttest.NewNopHost(), component.TelemetrySettings{}) + c, err = hcs.ToClient(componenttest.NewNopHost(), + component.TelemetrySettings{ + TelemetrySettingsBase: &component.TelemetrySettingsBase{}, + }, + ) require.NoError(b, err) } for pb.Next() { diff --git a/service/extensions/extensions.go b/service/extensions/extensions.go index b0f742b3f542..57601c8a5100 100644 --- a/service/extensions/extensions.go +++ b/service/extensions/extensions.go @@ -66,13 +66,22 @@ func (bes *Extensions) Shutdown(ctx context.Context) error { extID := bes.extensionIDs[i] instanceID := bes.instanceIDs[extID] ext := bes.extMap[extID] - _ = bes.telemetry.Status.ReportComponentStatus(instanceID, component.NewStatusEvent(component.StatusStopping)) + _ = bes.telemetry.Status.ReportComponentStatus( + instanceID, + component.NewStatusEvent(component.StatusStopping), + ) if err := ext.Shutdown(ctx); err != nil { - _ = bes.telemetry.Status.ReportComponentStatus(instanceID, component.NewPermanentErrorEvent(err)) + _ = bes.telemetry.Status.ReportComponentStatus( + instanceID, + component.NewPermanentErrorEvent(err), + ) errs = multierr.Append(errs, err) continue } - _ = bes.telemetry.Status.ReportComponentStatus(instanceID, component.NewStatusEvent(component.StatusStopped)) + _ = bes.telemetry.Status.ReportComponentStatus( + instanceID, + component.NewStatusEvent(component.StatusStopped), + ) } return errs diff --git a/service/internal/graph/graph.go b/service/internal/graph/graph.go index 73e5baeccab4..02c2da92c314 100644 --- a/service/internal/graph/graph.go +++ b/service/internal/graph/graph.go @@ -429,15 +429,24 @@ func (g *Graph) ShutdownAll(ctx context.Context) error { } instanceID := g.instanceIDs[node.ID()] - _ = g.telemetry.Status.ReportComponentStatus(instanceID, component.NewStatusEvent(component.StatusStopping)) + _ = g.telemetry.Status.ReportComponentStatus( + instanceID, + component.NewStatusEvent(component.StatusStopping), + ) if compErr := comp.Shutdown(ctx); compErr != nil { errs = multierr.Append(errs, compErr) - _ = g.telemetry.Status.ReportComponentStatus(instanceID, component.NewPermanentErrorEvent(compErr)) + _ = g.telemetry.Status.ReportComponentStatus( + instanceID, + component.NewPermanentErrorEvent(compErr), + ) continue } - _ = g.telemetry.Status.ReportComponentStatus(instanceID, component.NewStatusEvent(component.StatusStopped)) + _ = g.telemetry.Status.ReportComponentStatus( + instanceID, + component.NewStatusEvent(component.StatusStopped), + ) } return errs } diff --git a/service/internal/servicetelemetry/telemetry_settings.go b/service/internal/servicetelemetry/telemetry_settings.go index 11ea8306440d..471d5079f13b 100644 --- a/service/internal/servicetelemetry/telemetry_settings.go +++ b/service/internal/servicetelemetry/telemetry_settings.go @@ -8,9 +8,9 @@ import ( "go.opentelemetry.io/collector/service/internal/status" ) -// TelemetrySettings mirrors component.TelemetrySettings except for the method signature of -// ReportComponentStatus. The service level TelemetrySettings is not bound a specific component, and -// therefore takes a component.InstanceID as an argument. +// TelemetrySettings mirrors component.TelemetrySettings except for the mechanism for reporting +// status. Service-level status reporting has additional methods which can report status for +// components by their InstanceID whereas the component versions are tied to a specific component. type TelemetrySettings struct { *component.TelemetrySettingsBase Status *status.Reporter diff --git a/service/internal/status/status.go b/service/internal/status/status.go index 145aa43ec17e..f97256194628 100644 --- a/service/internal/status/status.go +++ b/service/internal/status/status.go @@ -100,7 +100,7 @@ type Reporter struct { onStatusChange NotifyStatusFunc } -// NewReporter a reporter that will invoke the NotifyStatusFunc when a component's status +// NewReporter returns a reporter that will invoke the NotifyStatusFunc when a component's status // has changed. func NewReporter(onStatusChange NotifyStatusFunc) *Reporter { return &Reporter{ @@ -148,6 +148,7 @@ func (r *Reporter) ReportComponentStatusIf( return nil } +// Note: a lock must be acquired before calling this method. func (r *Reporter) componentFSM(id *component.InstanceID) *fsm { fsm, ok := r.fsmMap[id] if !ok {