From e69f2f38ffd5147337d0d8a1bec1c8eb076e9e0f Mon Sep 17 00:00:00 2001 From: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Date: Tue, 1 Oct 2024 09:27:59 -0600 Subject: [PATCH] [componentstatus] Continue DataType rename (#11313) #### Description Continues the DataType rename process for `NewInstanceIDWithPipelineIDs`, `AllPipelineIDsWithPipelineIDs`, and `WithPipelineIDs`. #### Link to tracking issue Related to https://github.com/open-telemetry/opentelemetry-collector/issues/9429 --- .../componentstatus-continue-rename.yaml | 25 +++++++ component/componentstatus/instance.go | 69 ++++--------------- component/componentstatus/instance_test.go | 22 +++--- internal/e2e/status_test.go | 2 +- service/extensions/extensions.go | 2 +- service/internal/graph/graph.go | 14 ++-- service/internal/graph/graph_test.go | 12 ++-- 7 files changed, 66 insertions(+), 80 deletions(-) create mode 100644 .chloggen/componentstatus-continue-rename.yaml diff --git a/.chloggen/componentstatus-continue-rename.yaml b/.chloggen/componentstatus-continue-rename.yaml new file mode 100644 index 00000000000..abe413843c5 --- /dev/null +++ b/.chloggen/componentstatus-continue-rename.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: deprecation + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: componentstatus + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Deprecated `NewInstanceIDWithPipelineIDs`, `AllPipelineIDsWithPipelineIDs`, and `WithPipelineIDs`. Use `NewInstanceID`, `AllPipelineIDs`, and `WithPipelines` instead. + +# One or more tracking issues or pull requests related to the change +issues: [11313] + +# (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. +# Use pipe (|) for multiline entries. +subtext: + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [api] \ No newline at end of file diff --git a/component/componentstatus/instance.go b/component/componentstatus/instance.go index 96475510fef..1485ba15a4b 100644 --- a/component/componentstatus/instance.go +++ b/component/componentstatus/instance.go @@ -27,25 +27,20 @@ type InstanceID struct { } // NewInstanceID returns an ID that uniquely identifies a component. -// -// Deprecated: [v0.110.0] Use NewInstanceIDWithPipelineID instead -func NewInstanceID(componentID component.ID, kind component.Kind, pipelineIDs ...component.ID) *InstanceID { +func NewInstanceID(componentID component.ID, kind component.Kind, pipelineIDs ...pipeline.ID) *InstanceID { instanceID := &InstanceID{ componentID: componentID, kind: kind, } - instanceID.addPipelines(convertToPipelineIDs(pipelineIDs)) + instanceID.addPipelines(pipelineIDs) return instanceID } // NewInstanceIDWithPipelineIDs returns an InstanceID that uniquely identifies a component. +// +// Deprecated: [v0.111.0] Use NewInstanceIDWithPipelineID instead func NewInstanceIDWithPipelineIDs(componentID component.ID, kind component.Kind, pipelineIDs ...pipeline.ID) *InstanceID { - instanceID := &InstanceID{ - componentID: componentID, - kind: kind, - } - instanceID.addPipelines(pipelineIDs) - return instanceID + return NewInstanceID(componentID, kind, pipelineIDs...) } // ComponentID returns the ComponentID associated with this instance. @@ -60,16 +55,14 @@ func (id *InstanceID) Kind() component.Kind { // AllPipelineIDs calls f for each pipeline this instance is associated with. If // f returns false it will stop iteration. -// -// Deprecated: [v0.110.0] Use AllPipelineIDsWithPipelineIDs instead. -func (id *InstanceID) AllPipelineIDs(f func(component.ID) bool) { +func (id *InstanceID) AllPipelineIDs(f func(pipeline.ID) bool) { var bs []byte for _, b := range []byte(id.pipelineIDs) { if b != pipelineDelim { bs = append(bs, b) continue } - pipelineID := component.ID{} + pipelineID := pipeline.ID{} err := pipelineID.UnmarshalText(bs) bs = bs[:0] if err != nil { @@ -83,49 +76,30 @@ func (id *InstanceID) AllPipelineIDs(f func(component.ID) bool) { // AllPipelineIDsWithPipelineIDs calls f for each pipeline this instance is associated with. If // f returns false it will stop iteration. +// +// Deprecated: [v0.111.0] Use AllPipelineIDs instead. func (id *InstanceID) AllPipelineIDsWithPipelineIDs(f func(pipeline.ID) bool) { - var bs []byte - for _, b := range []byte(id.pipelineIDs) { - if b != pipelineDelim { - bs = append(bs, b) - continue - } - pipelineID := pipeline.ID{} - err := pipelineID.UnmarshalText(bs) - bs = bs[:0] - if err != nil { - continue - } - if !f(pipelineID) { - break - } - } + id.AllPipelineIDs(f) } // WithPipelines returns a new InstanceID updated to include the given // pipelineIDs. -// -// Deprecated: [v0.110.0] Use WithPipelineIDs instead -func (id *InstanceID) WithPipelines(pipelineIDs ...component.ID) *InstanceID { +func (id *InstanceID) WithPipelines(pipelineIDs ...pipeline.ID) *InstanceID { instanceID := &InstanceID{ componentID: id.componentID, kind: id.kind, pipelineIDs: id.pipelineIDs, } - instanceID.addPipelines(convertToPipelineIDs(pipelineIDs)) + instanceID.addPipelines(pipelineIDs) return instanceID } // WithPipelineIDs returns a new InstanceID updated to include the given // pipelineIDs. +// +// Deprecated: [v0.111.0] Use WithPipelines instead func (id *InstanceID) WithPipelineIDs(pipelineIDs ...pipeline.ID) *InstanceID { - instanceID := &InstanceID{ - componentID: id.componentID, - kind: id.kind, - pipelineIDs: id.pipelineIDs, - } - instanceID.addPipelines(pipelineIDs) - return instanceID + return id.WithPipelines(pipelineIDs...) } func (id *InstanceID) addPipelines(pipelineIDs []pipeline.ID) { @@ -138,16 +112,3 @@ func (id *InstanceID) addPipelines(pipelineIDs []pipeline.ID) { strIDs = slices.Compact(strIDs) id.pipelineIDs = strings.Join(strIDs, delim) + delim } - -func convertToPipelineIDs(ids []component.ID) []pipeline.ID { - pipelineIDs := make([]pipeline.ID, len(ids)) - for i, id := range ids { - if id.Name() != "" { - pipelineIDs[i] = pipeline.MustNewIDWithName(id.Type().String(), id.Name()) - } else { - pipelineIDs[i] = pipeline.MustNewID(id.Type().String()) - } - - } - return pipelineIDs -} diff --git a/component/componentstatus/instance_test.go b/component/componentstatus/instance_test.go index 7c8b0d4691a..6a7edeb1dfa 100644 --- a/component/componentstatus/instance_test.go +++ b/component/componentstatus/instance_test.go @@ -18,13 +18,13 @@ func TestInstanceID(t *testing.T) { tracesB := pipeline.MustNewIDWithName("traces", "b") tracesC := pipeline.MustNewIDWithName("traces", "c") - idTracesA := NewInstanceIDWithPipelineIDs(traces, component.KindReceiver, tracesA) - idTracesAll := NewInstanceIDWithPipelineIDs(traces, component.KindReceiver, tracesA, tracesB, tracesC) + idTracesA := NewInstanceID(traces, component.KindReceiver, tracesA) + idTracesAll := NewInstanceID(traces, component.KindReceiver, tracesA, tracesB, tracesC) assert.NotEqual(t, idTracesA, idTracesAll) assertHasPipelines := func(t *testing.T, instanceID *InstanceID, expectedPipelineIDs []pipeline.ID) { var pipelineIDs []pipeline.ID - instanceID.AllPipelineIDsWithPipelineIDs(func(id pipeline.ID) bool { + instanceID.AllPipelineIDs(func(id pipeline.ID) bool { pipelineIDs = append(pipelineIDs, id) return true }) @@ -40,25 +40,25 @@ func TestInstanceID(t *testing.T) { { name: "equal instances", id1: idTracesA, - id2: NewInstanceIDWithPipelineIDs(traces, component.KindReceiver, tracesA), + id2: NewInstanceID(traces, component.KindReceiver, tracesA), pipelineIDs: []pipeline.ID{tracesA}, }, { name: "equal instances - out of order", id1: idTracesAll, - id2: NewInstanceIDWithPipelineIDs(traces, component.KindReceiver, tracesC, tracesB, tracesA), + id2: NewInstanceID(traces, component.KindReceiver, tracesC, tracesB, tracesA), pipelineIDs: []pipeline.ID{tracesA, tracesB, tracesC}, }, { name: "with pipelines", id1: idTracesAll, - id2: idTracesA.WithPipelineIDs(tracesB, tracesC), + id2: idTracesA.WithPipelines(tracesB, tracesC), pipelineIDs: []pipeline.ID{tracesA, tracesB, tracesC}, }, { name: "with pipelines - out of order", id1: idTracesAll, - id2: idTracesA.WithPipelineIDs(tracesC, tracesB), + id2: idTracesA.WithPipelines(tracesC, tracesB), pipelineIDs: []pipeline.ID{tracesA, tracesB, tracesC}, }, } { @@ -70,8 +70,8 @@ func TestInstanceID(t *testing.T) { } } -func TestAllPipelineIDsWithPipelineIDs(t *testing.T) { - instanceID := NewInstanceIDWithPipelineIDs( +func TestAllPipelineIDs(t *testing.T) { + instanceID := NewInstanceID( component.MustNewID("traces"), component.KindReceiver, pipeline.MustNewIDWithName("traces", "a"), @@ -80,14 +80,14 @@ func TestAllPipelineIDsWithPipelineIDs(t *testing.T) { ) count := 0 - instanceID.AllPipelineIDsWithPipelineIDs(func(pipeline.ID) bool { + instanceID.AllPipelineIDs(func(pipeline.ID) bool { count++ return true }) assert.Equal(t, 3, count) count = 0 - instanceID.AllPipelineIDsWithPipelineIDs(func(pipeline.ID) bool { + instanceID.AllPipelineIDs(func(pipeline.ID) bool { count++ return false }) diff --git a/internal/e2e/status_test.go b/internal/e2e/status_test.go index a87a59807f9..67dc156804b 100644 --- a/internal/e2e/status_test.go +++ b/internal/e2e/status_test.go @@ -128,7 +128,7 @@ func Test_ComponentStatusReporting_SharedInstance(t *testing.T) { for instanceID, events := range eventsReceived { pipelineIDs := "" - instanceID.AllPipelineIDsWithPipelineIDs(func(id pipeline.ID) bool { + instanceID.AllPipelineIDs(func(id pipeline.ID) bool { pipelineIDs += id.String() + "," return true }) diff --git a/service/extensions/extensions.go b/service/extensions/extensions.go index 140d8b3b6ab..fcefcb33f20 100644 --- a/service/extensions/extensions.go +++ b/service/extensions/extensions.go @@ -209,7 +209,7 @@ func New(ctx context.Context, set Settings, cfg Config, options ...Option) (*Ext } for _, extID := range cfg { - instanceID := componentstatus.NewInstanceIDWithPipelineIDs(extID, component.KindExtension) + instanceID := componentstatus.NewInstanceID(extID, component.KindExtension) extSet := extension.Settings{ ID: extID, TelemetrySettings: set.Telemetry, diff --git a/service/internal/graph/graph.go b/service/internal/graph/graph.go index 132f58c6103..3e0c1e5d057 100644 --- a/service/internal/graph/graph.go +++ b/service/internal/graph/graph.go @@ -200,11 +200,11 @@ func (g *Graph) createReceiver(pipelineID pipeline.ID, recvID component.ID) *rec rcvrNode := newReceiverNode(pipelineID.Signal(), recvID) if node := g.componentGraph.Node(rcvrNode.ID()); node != nil { instanceID := g.instanceIDs[node.ID()] - g.instanceIDs[node.ID()] = instanceID.WithPipelineIDs(pipelineID) + g.instanceIDs[node.ID()] = instanceID.WithPipelines(pipelineID) return node.(*receiverNode) } g.componentGraph.AddNode(rcvrNode) - g.instanceIDs[rcvrNode.ID()] = componentstatus.NewInstanceIDWithPipelineIDs( + g.instanceIDs[rcvrNode.ID()] = componentstatus.NewInstanceID( recvID, component.KindReceiver, pipelineID, ) return rcvrNode @@ -213,7 +213,7 @@ func (g *Graph) createReceiver(pipelineID pipeline.ID, recvID component.ID) *rec func (g *Graph) createProcessor(pipelineID pipeline.ID, procID component.ID) *processorNode { procNode := newProcessorNode(pipelineID, procID) g.componentGraph.AddNode(procNode) - g.instanceIDs[procNode.ID()] = componentstatus.NewInstanceIDWithPipelineIDs( + g.instanceIDs[procNode.ID()] = componentstatus.NewInstanceID( procID, component.KindProcessor, pipelineID, ) return procNode @@ -223,11 +223,11 @@ func (g *Graph) createExporter(pipelineID pipeline.ID, exprID component.ID) *exp expNode := newExporterNode(pipelineID.Signal(), exprID) if node := g.componentGraph.Node(expNode.ID()); node != nil { instanceID := g.instanceIDs[expNode.ID()] - g.instanceIDs[expNode.ID()] = instanceID.WithPipelineIDs(pipelineID) + g.instanceIDs[expNode.ID()] = instanceID.WithPipelines(pipelineID) return node.(*exporterNode) } g.componentGraph.AddNode(expNode) - g.instanceIDs[expNode.ID()] = componentstatus.NewInstanceIDWithPipelineIDs( + g.instanceIDs[expNode.ID()] = componentstatus.NewInstanceID( expNode.componentID, component.KindExporter, pipelineID, ) return expNode @@ -237,11 +237,11 @@ func (g *Graph) createConnector(exprPipelineID, rcvrPipelineID pipeline.ID, conn connNode := newConnectorNode(exprPipelineID.Signal(), rcvrPipelineID.Signal(), connID) if node := g.componentGraph.Node(connNode.ID()); node != nil { instanceID := g.instanceIDs[connNode.ID()] - g.instanceIDs[connNode.ID()] = instanceID.WithPipelineIDs(exprPipelineID, rcvrPipelineID) + g.instanceIDs[connNode.ID()] = instanceID.WithPipelines(exprPipelineID, rcvrPipelineID) return node.(*connectorNode) } g.componentGraph.AddNode(connNode) - g.instanceIDs[connNode.ID()] = componentstatus.NewInstanceIDWithPipelineIDs( + g.instanceIDs[connNode.ID()] = componentstatus.NewInstanceID( connNode.componentID, component.KindConnector, exprPipelineID, rcvrPipelineID, ) return connNode diff --git a/service/internal/graph/graph_test.go b/service/internal/graph/graph_test.go index ea58f938130..1962bf9e49f 100644 --- a/service/internal/graph/graph_test.go +++ b/service/internal/graph/graph_test.go @@ -2796,12 +2796,12 @@ func TestStatusReportedOnStartupShutdown(t *testing.T) { eSdErr := &testNode{id: component.MustNewIDWithName("e_sd_err", "1"), shutdownErr: assert.AnError} instanceIDs := map[*testNode]*componentstatus.InstanceID{ - rNoErr: componentstatus.NewInstanceIDWithPipelineIDs(rNoErr.id, component.KindReceiver), - rStErr: componentstatus.NewInstanceIDWithPipelineIDs(rStErr.id, component.KindReceiver), - rSdErr: componentstatus.NewInstanceIDWithPipelineIDs(rSdErr.id, component.KindReceiver), - eNoErr: componentstatus.NewInstanceIDWithPipelineIDs(eNoErr.id, component.KindExporter), - eStErr: componentstatus.NewInstanceIDWithPipelineIDs(eStErr.id, component.KindExporter), - eSdErr: componentstatus.NewInstanceIDWithPipelineIDs(eSdErr.id, component.KindExporter), + rNoErr: componentstatus.NewInstanceID(rNoErr.id, component.KindReceiver), + rStErr: componentstatus.NewInstanceID(rStErr.id, component.KindReceiver), + rSdErr: componentstatus.NewInstanceID(rSdErr.id, component.KindReceiver), + eNoErr: componentstatus.NewInstanceID(eNoErr.id, component.KindExporter), + eStErr: componentstatus.NewInstanceID(eStErr.id, component.KindExporter), + eSdErr: componentstatus.NewInstanceID(eSdErr.id, component.KindExporter), } // compare two maps of status events ignoring timestamp