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 Run should exit quickly if shutdown was called prior #4946

Closed
cpheps opened this issue Mar 2, 2022 · 8 comments
Closed

Collector Run should exit quickly if shutdown was called prior #4946

cpheps opened this issue Mar 2, 2022 · 8 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Good issue for contributors to OpenTelemetry Service to pick up priority:p2 Medium

Comments

@cpheps
Copy link
Contributor

cpheps commented Mar 2, 2022

Describe the bug
Currently if Shutdown is called before Run all this startup code runs before runAndWaitForShutdownEvent which will then process the shutdown.

Steps to reproduce

  1. Created collector
  2. Call Shutdown
  3. Call Run

What did you expect to see?
Collector not attempt to run startup/configure code and exit early.

What did you see instead?
Startup code was run then exited.

What version did you use?
Version: v0.45.0

What config did you use?
Config: any valid config

Environment
OS: MacOS 12.2
Compiler(if manually compiled): go 1.17.6

Additional context
Action item from #4878

@cpheps cpheps added the bug Something isn't working label Mar 2, 2022
@dmitryax
Copy link
Member

cc @djaglowski

@bogdandrutu bogdandrutu added help wanted Good issue for contributors to OpenTelemetry Service to pick up good first issue Good for newcomers priority:p2 Medium labels Apr 8, 2022
@Chinwendu20
Copy link
Contributor

Could you please assign this to me?

@Chinwendu20
Copy link
Contributor

According to the code comments, run should not be called after shutdown

image

@cpheps
Copy link
Contributor Author

cpheps commented Oct 10, 2022

@Chinwendu20 This ticket is to basically make a short circuit path in Run where if a given instance of a Collector is Shutdown then subsequent calls to run should detect this and exit quickly. There's a lot of logic that happens in Run before it would detect that it was already shut down.

I think there's a valid question on if Run should return and error if it's used after Shutdown is called. I believe it should as it would provide helpful context for the caller as to why didn't execute properly.

@cpheps
Copy link
Contributor Author

cpheps commented Oct 10, 2022

@dmitryax Can you assign @Chinwendu20 to this issue please?

Chinwendu20 added a commit to Chinwendu20/opentelemetry-collector that referenced this issue Oct 11, 2022
Signed-off-by: Maureen <amaka013@gmail.com>
Chinwendu20 added a commit to Chinwendu20/opentelemetry-collector that referenced this issue Oct 11, 2022
Signed-off-by: Maureen <amaka013@gmail.com>
Chinwendu20 added a commit to Chinwendu20/opentelemetry-collector that referenced this issue Oct 12, 2022
Signed-off-by: Maureen <amaka013@gmail.com>
Chinwendu20 added a commit to Chinwendu20/opentelemetry-collector that referenced this issue Oct 13, 2022
Chinwendu20 added a commit to Chinwendu20/opentelemetry-collector that referenced this issue Oct 13, 2022
@jpkrohling
Copy link
Member

Collector not attempt to run startup/configure code and exit early.

Is this even what we want? I was under the impression that we should be able to stop/start the collector whenever we wanted.

@tigrannajaryan, I'm pretty sure I got this from you a few years ago :-) Do you think that's still valid to this day?

@cpheps
Copy link
Contributor Author

cpheps commented Nov 28, 2022

@jpkrohling I was under the impression you could start multiple instance of the collector per process but not a single instance over and over. This would make sure a single instance exits early rather than doing a full startup before closing.

@atoulme
Copy link
Contributor

atoulme commented Dec 19, 2023

The comment on Run now says:

// Consecutive calls to Run are not allowed, Run shouldn't be called once a collector is shut down.

See

// Consecutive calls to Run are not allowed, Run shouldn't be called once a collector is shut down.

I believe this means the use case you describe is not supported. I propose to close this issue as not planned, please comment or reopen the issue if you disagree.

@atoulme atoulme closed this as not planned Won't fix, can't repro, duplicate, stale Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Good issue for contributors to OpenTelemetry Service to pick up priority:p2 Medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants