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

Collector Lifecycle Inconsistencies #4826

Closed
cpheps opened this issue Feb 9, 2022 · 1 comment · Fixed by #4974
Closed

Collector Lifecycle Inconsistencies #4826

cpheps opened this issue Feb 9, 2022 · 1 comment · Fixed by #4974
Labels
bug Something isn't working

Comments

@cpheps
Copy link
Contributor

cpheps commented Feb 9, 2022

Describe the bug
There are a few lifecycle inconsistencies in the collector lifecycle management.

  1. In Collector.Run the collector state is not updated if the collector errors on startup. Leaving the collector in a Starting state even though Run has excited with and error.
  2. There's a race condition/panic in Collector.Shutdown if Shutdown is called before the logger is initialized it will panic in the Shutdown recover.

Steps to reproduce

  1. Call Collector.Run with a bad config and then call Collector.GetState to observe state is still Starting.
  2. It's harder to reliably trigger the race condition but if you call Collector.Run with a bad config then call Collector.Shutdown it will have the same panic.

What did you expect to see?

  1. State updates correctly changed to Closed or maybe a new Failed or Error state to indicate a failed startup.
  2. Shutdown never panics no matter when it's called.

What did you see instead?

  1. State is Started
  2. Shutdown panics

What version did you use?
v0.43.0

What config did you use?
Any invalid configuration will work.

Environment
OS: macOS Monterey (12.2)
Compiler: go 1.17.6

Additional context
I'd be happy to submit a PR to fix the above I think the only open question is if the collector errors on startup should the state be Closed or should we introduce a new state to represent a failure?

It's a bit unclear how Collector.Run and Collector.GetState are supposed to be used by the caller. It seems like Run should be called an expected to block until the collector finishes (with or without an error). It's not clear if GetState is meant to be constantly monitored to help with managing a collector or if it's purely informational on what the collector is doing.

@cpheps cpheps added the bug Something isn't working label Feb 9, 2022
@cpheps
Copy link
Contributor Author

cpheps commented Feb 9, 2022

Created a PR to fix the Shutdown race condition as that was straight forward and didn't require discussion. Keeping issue open as there still needs to be a decision on the collectors state when it fails to startup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant