feat/refactor: Replace Logger module with tracing_subscriber + tracing libs #135
Conversation
This file contains hidden or 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
This commit introduces tracing, tracing_subscriber and tracing_appender libs. The objective is replace the current logging in-house module, for a more extensible architecture which supports the same features as the current logging method, but which is simpler to build, maintain, and well known in the community. Signed-off-by: Lorenzo Bertin Salvador <[email protected]>
This commit changes the new logging logic so logs are stored in the path defined at patch-hub's config. Since logging should encompass the whole application lifecycle, we have to correctly deal with logs produced before/during Config initialization. To solve this firstly we save the logs to a temporary path, and then, after Config intitialization we move the existing logs to the path defined at the Config and reload the logging layer so new logs are stored in the updated path. Signed-off-by: Lorenzo Bertin Salvador <[email protected]>
This commit is just a way of testing the log behavior regarding logs created before and after config initialization Signed-off-by: Lorenzo Bertin Salvador <[email protected]>
…OPPED This reverts commit 948f13d0be0770509fdcdc62bf89a70dede6db3f.
This commit finishes making patch-hub new log logic equal to the previous one (with the Logger module). The log file names are the same and every log is produced by tracing::event! macro. Signed-off-by: Lorenzo Bertin Salvador <[email protected]>
The garbage_collector function continues to work with the new logging methods since it just depends on log path and file names which didn't change. Temporary logs are on /tmp which is already a temporary directory, so garbage_collector doesn't need to deal with them. The log_on_error macro was already adapted to deal with tracing::Level log level. Signed-off-by: Lorenzo Bertin Salvador <[email protected]>
The module occurrences and usage was already replaced in previous commits, so it's possible to remove the whole module. Signed-off-by: Lorenzo Bertin Salvador <[email protected]>
lorenzoberts
changed the title
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.
One of the plans for patch-hub is to support generating metrics—both usage metrics and product-specific metrics—so that it's possible to analyze user interaction with the application and identify potential performance bottlenecks. To collect these metrics (or any kind of telemetry, really), we should use the OTLP protocol integration with Rust. One way to achieve this is through the opentelemetry-rust crate along with tracing_subscriber.
With that in mind, if we were to follow the same pattern we use for collecting application logs, we would need to create a new module for telemetry, and we’d have to manage both the instrumentation and the instantiation of the metrics in-house. Instead, I believe the better approach is to start using the well-known crates tracing and tracing_subscriber for both logs and metrics. This PR is a proof of concept for that possibility—still without introducing anything directly related to telemetry, only replacing our current logging setup.
The PR is split into a few commits, and it's worth noting that two of them should be dropped before merging. I created 0ee020a23d64d50b3963a9cace39c118dab517a6 just to make it easier to validate the logging behavior.
As always, I'm totally open for new ideas and suggestions. Let me know what you think.