Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[component helper] Adding lifecycle state management to *helper. #6562

Conversation

MovieStoreGuy
Copy link
Contributor

Description:

Extending the current helpers to adhere to the current component expectations

Link to tracking Issue:
#6507

Testing:

Haven't extended the tests yet, but my intention is to extend the lifecycle test to check if a component can:

  • Allow for partial shutdown
  • is restartable

Documentation:
Haven't added it yet, just wanting to get feedback to check the approach.

Adding in lifecycle for the components to abstract state management for
the components
@MovieStoreGuy
Copy link
Contributor Author

The current components Processor, Extension don't have helpers at this point, is worth me adding it as part of this PR?

Comment on lines +19 to +30
// Lifecycle represents the current
// running state of the component
// that is intended to be concurrency safe.
type State interface {
// Start atomically updates the state and returns true if
// the previous state was not running.
Start() (started bool)

// Stop atomically updates the state and returns true if
// the previous state was not shutdown.
Stop() (stopped bool)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Lifecycle represents the current
// running state of the component
// that is intended to be concurrency safe.
type State interface {
// Start atomically updates the state and returns true if
// the previous state was not running.
Start() (started bool)
// Stop atomically updates the state and returns true if
// the previous state was not shutdown.
Stop() (stopped bool)
}
// This will implement component.Component.
type Component struct {
current *atomic.Int64
component.StartFunc
component.ShutdownFunc
}
func NewComponent(start component.StartFunc, shutdown component.ShutdownFunc) *Component {
return &Component{
current: atomic.NewInt64(shutdown),
StartFunc: start,
ShutdownFunc: shutdown
}
}
func (c *Component) Start(ctx context.Context, host component.Host) error {
if s.current.Swap(running) != shutdown {
return nil
}
c.StartFunc.Start(ctx, host)
}
func (c *Component) Shutdown(ctx context.Context) error {
if s.current.Swap(running) != running {
return nil
}
c.ShutdownFunc.Shutdown(ctx)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then in the exporterhelper can have

type baseExporter struct {
	*lifecyle.Component
}

@github-actions
Copy link
Contributor

github-actions bot commented Dec 2, 2022

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Dec 2, 2022
@bogdandrutu
Copy link
Member

The current components Processor, Extension don't have helpers at this point, is worth me adding it as part of this PR?

https://github.com/open-telemetry/opentelemetry-collector/blob/main/processor/processorhelper/traces.go#L39

@github-actions github-actions bot removed the Stale label Dec 3, 2022
@MovieStoreGuy
Copy link
Contributor Author

Sorry @bogdandrutu , I've moved the PR over to #6702 since I messed up my local git.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants