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 Shutdown should block until Run cleans up #4947

Open
cpheps opened this issue Mar 2, 2022 · 7 comments
Open

Collector Shutdown should block until Run cleans up #4947

cpheps opened this issue Mar 2, 2022 · 7 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

Is your feature request related to a problem? Please describe.
Discussion from 4878 led to a decision to design Shutdown to follow language patterns for how it and Run should operate. Following http.Server as an example the following behaviors should be met:

  1. Shutdown be safe to be called at any point. Even if there is no action to take
  2. If Shutdown is called before Run it will exit cleanly and the next call to Run will exit due to the previous Shutdown
  3. If Shutdown is called while Run is active it should block until Run has confirmed shutdown.

Point 3 isn't currently implemented but desired behavior.

Describe the solution you'd like
Shutdown will block until Run has confirmed cleanup. Whatever Shutdown checks for should not stop it from cleanly exiting if Run hasn't been called yet or after Run was called.

Describe alternatives you've considered
Alternative is don't block Shutdown and just exit like it does now after closing the channel.

Additional context
Action item from #4878

If we want to go this route I think there needs to be some discussion on what Shutdown should block on. It seems like it should be when the state is Closing but that won't allow Shutdown to cleanly exit if called before or after Run.

@dmitryax dmitryax added bug Something isn't working priority:p2 Medium labels Mar 25, 2022
@dmitryax
Copy link
Member

@djaglowski said it can be upstreamed

@djaglowski
Copy link
Member

Following up on this - I misunderstood the context of this issue, #4946, and 4939, and mistakenly believed they may have been resolved as part of #4878.

@bogdandrutu bogdandrutu added help wanted Good issue for contributors to OpenTelemetry Service to pick up good first issue Good for newcomers labels Apr 8, 2022
@deepto98
Copy link

deepto98 commented Jul 4, 2022

Can I work on this?

@dmitryax
Copy link
Member

dmitryax commented Jul 4, 2022

@deepto98 sure. Assigned to you. Thanks

@Chinwendu20
Copy link
Contributor

Hi @deepto98, are you still working on this? I would like to take this up

@Chinwendu20
Copy link
Contributor

Hi @cpheps I guess 1 and 2 has been addressed by other the other issues

@michalpristas
Copy link

going to take a look at this

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

No branches or pull requests

7 participants