Skip to content

Commit

Permalink
[componentstatus] Continue DataType rename (open-telemetry#11313)
Browse files Browse the repository at this point in the history 
#### Description
Continues the DataType rename process for
`NewInstanceIDWithPipelineIDs`, `AllPipelineIDsWithPipelineIDs`, and
`WithPipelineIDs`.

#### Link to tracking issue
Related to
open-telemetry#9429
  • Loading branch information
@TylerHelmuth @jackgopack4
TylerHelmuth authored and jackgopack4 committed Oct 8, 2024
1 parent a3bca7b commit c4160ef
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 80 deletions.
25 changes: 25 additions & 0 deletions .chloggen/componentstatus-continue-rename.yaml
View file
Original file line number Diff line number Diff line change
@@ -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]
69 changes: 15 additions & 54 deletions component/componentstatus/instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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) {
Expand All @@ -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
}
22 changes: 11 additions & 11 deletions component/componentstatus/instance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand All @@ -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},
},
} {
Expand All @@ -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"),
Expand All @@ -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
})
Expand Down
2 changes: 1 addition & 1 deletion internal/e2e/status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down
2 changes: 1 addition & 1 deletion service/extensions/extensions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 7 additions & 7 deletions service/internal/graph/graph.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions service/internal/graph/graph_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c4160ef

Please sign in to comment.