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

Add debug logging #285

Closed
wants to merge 17 commits into from
Closed

Add debug logging #285

wants to merge 17 commits into from

Conversation

diehuxx
Copy link
Contributor

@diehuxx diehuxx commented Aug 15, 2024

Add macro for debug logging and env var to github actions, patterned off tbdex-rs (https://github.com/TBD54566975/tbdex-rs/pull/111/files). Addresses the line of this issue "Add debug logging like we did over in tbdex-rs" #277.

Open Question: In the line "Add version to debug logs" of this issue #277, does that refer to the web5 crate version?

@KendallWeihe
Copy link
Contributor

Add version to debug logs

What I really want is the git commit sha, but that would be require integration into the pipeline building. I would settle for the crate version but that's less good because there are less assurances... that could easily fall through the cracks. Anyways, neither are strictly necessary at this moment, @diehuxx could you create a Issue on the backlog with all of the necessary context for this?

@KendallWeihe
Copy link
Contributor

Adding a git commit hash in the debug log is the most bulletproof way to debug production issues at scale. Because then you just open git and you have strong assurances you're reading the source code of the exact artifact that is running.

@@ -15,6 +15,7 @@ josekit = "0.8.6"
jsonpath-rust = "0.5.1"
jsonschema = { version = "0.18.0", default-features = false }
k256 = { version = "0.13.3", features = ["ecdsa", "jwk"] }
lazy_static = "1.5.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we update the Justfile to 1.80.0 and use the new LazyLock? I'm doing that over here https://github.com/TBD54566975/web5-rs/pull/283/files#diff-8b2fda406bc73cf365197448d20bd9ea1d05db17e9b8dca69881ad9dd9755f5d (scroll down to the tests)

crates/web5/build.rs Outdated Show resolved Hide resolved
Diane Huxley added 3 commits August 15, 2024 13:22
* main:
  Add Web5 Test Vectors and CI (#276)
  Build byte code files in CI, remove from git tree (#279)
@@ -4,3 +4,30 @@ pub mod dids;

#[cfg(test)]
mod test_helpers;

lazy_static::lazy_static! {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we use the LazyLock or LazyCell here and then remove the addition of lazy_static in the Cargo.toml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upd8d

KendallWeihe and others added 2 commits August 19, 2024 13:30
* Add web5_proc_macros with git_sha proc macro

* Use short SHA, case insensitive log level, remove kt code

* PR review

* Fix linting, fix linux builds

* Fix CLI build
Diane Huxley added 5 commits August 19, 2024 16:48
* debug-logging-include-str:
  Remove git sha compiler flag
  git ignore git sha resource
  Remove git hash from kotlin logger
  Build resources file with git SHA
@diehuxx diehuxx closed this Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants