fix(server): better tracing management #543
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
robertohuertasm
force-pushed
the
rob/fix/log-to-files-IDE-3768
branch
from
68ef20a to
731c95b
Compare
robertohuertasm
force-pushed
the
rob/fix/log-to-files-IDE-3768
branch
from
350cb67 to
a2e57ec
Compare
amaanq
approved these changes
Nov 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem are you trying to solve?
There were different threads using process::exit. This was making it difficult to correctly flush the tracing messages.
On top of that, the
WorkerGuardwas shared but this was introducing other issues, as multiple references would avoid flushing the traces.In conclusion, the mixture of these two facts was making the tracing messages not being correctly flushed
What is your solution?
Centralized all the panics into the main thread so we can guarantee that the tracing messages are properly flushed by dropping the
WorkerGuardif needed.Alternatives considered
What the reviewer should know
There's only one panic left in the keep alive thread. This is a last resource mechanism, and it's only being used after having tried 2 different alternatives before.
While at the task I added errors 100, 102 and 103 so we can have more useful information if we find those codes in the IDE and handle those cases accordingly.
IDE-3768