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

Added CFE setting for logging span lifecycles #3936

Merged
merged 1 commit into from
Sep 4, 2023

Conversation

j4m1ef0rd
Copy link
Contributor

Pull Request

Closes: PRO-764

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works.
  • I have updated documentation where appropriate.

Summary

We were not able to find an easy solution for changing the span lifecycle logs during runtime (See Linier issue for details), so the best we can do is have it as a startup setting.
I added --logging.span_lifecycle command line arg that is false by default. This will stop all of the "enter/exit" log messages unless you restart your CFE with the setting enabled:

./target/debug/chainflip-engine --logging.span_lifecycle

@j4m1ef0rd j4m1ef0rd added the CFE label Sep 4, 2023
@j4m1ef0rd j4m1ef0rd self-assigned this Sep 4, 2023
@linear
Copy link

linear bot commented Sep 4, 2023

PRO-764 Make all tracing span levels TRACE

We currently have a lot of noise in the logs from tracing spans.

This is because many spans are declared as INFO- or DEBUG-level spans.

I would suggest we make all spans TRACE-level so that we have the option to log them selectively, but they are disabled by default.

@j4m1ef0rd j4m1ef0rd changed the title Added setting for logging span lifecycles Added CFE setting for logging span lifecycles Sep 4, 2023
@j4m1ef0rd j4m1ef0rd force-pushed the feat/log_span_lifecycle_setting branch from 02eee1f to 99d9c4c Compare September 4, 2023 06:12
@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Merging #3936 (99d9c4c) into main (a5205b9) will decrease coverage by 0%.
The diff coverage is 53%.

@@          Coverage Diff          @@
##            main   #3936   +/-   ##
=====================================
- Coverage     73%     72%   -0%     
=====================================
  Files        367     367           
  Lines      56037   56047   +10     
  Branches   56037   56047   +10     
=====================================
- Hits       40631   40629    -2     
- Misses     13345   13351    +6     
- Partials    2061    2067    +6     
Files Changed Coverage Δ
utilities/src/with_std.rs 52% <0%> (ø)
engine/src/settings.rs 82% <80%> (-<1%) ⬇️

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kylezs kylezs merged commit b195c43 into main Sep 4, 2023
42 checks passed
@kylezs kylezs deleted the feat/log_span_lifecycle_setting branch September 4, 2023 13:04
syan095 added a commit that referenced this pull request Sep 5, 2023
…on-integration

* origin/main:
  Added CFE setting for logging span lifecycles (#3936)
  fix: only burn flip if non zero (#3932)
  Fix: Correct Select Median Implementation (#3934)
  fix: independent witnessing startup (#3913)
  🍒 cherry-pick: changes in release for CI and chainspec (#3933)
  refactor: Re-arrange Chains traits for better composability (#3912)
  fix: log error when we try to transfer *more* than we have fetched (#3930)
  chore: add checks and increase timeout (#3928)
  Add `bind_redeem_address` to CLI (#3908)
  🍒 cherry-pick: add missing prod dockerfiles (#3926)
  chore: skip localnet specific tests in devnet 🤫 (#3919)
  fix: broadcast success should be witnessable after a rotation (#3921)

# Conflicts:
#	state-chain/cf-integration-tests/src/network.rs
dandanlen pushed a commit that referenced this pull request Sep 6, 2023
feat: added span setting and cmd line arg
dandanlen pushed a commit that referenced this pull request Sep 6, 2023
feat: added span setting and cmd line arg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants