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

feat: single thread mode #201

Merged
merged 3 commits into from
Feb 5, 2024
Merged

Conversation

andylokandy
Copy link
Collaborator

@andylokandy andylokandy commented Feb 4, 2024

Signed-off-by: Andy Lok [email protected]

Closes #197

Signed-off-by: Andy Lok <[email protected]>
@coveralls
Copy link

coveralls commented Feb 4, 2024

Pull Request Test Coverage Report for Build 7774880861

  • 0 of 19 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 78.749%

Totals Coverage Status
Change from base Build 7772646372: 0.04%
Covered Lines: 1712
Relevant Lines: 2174

💛 - Coveralls

@dotansimha
Copy link
Contributor

dotansimha commented Feb 4, 2024

Using this PR along with wasm32 runtime, I get time not implemented on this platform errors 🤔 Not sure what's the source of it. @andylokandy does the code in Span is using SystemTime somewhere? (or std::time::Instant?)

@dotansimha
Copy link
Contributor

dotansimha commented Feb 4, 2024

maybe this one? https://github.com/tikv/minitrace-rust/blob/master/minitrace/src/collector/global_collector.rs#L211

EDIT: Yeah that's the one :) I can confirm changing it does not fail, checking the Span collecting now @andylokandy

@dotansimha
Copy link
Contributor

dotansimha commented Feb 4, 2024

@andylokandy ok with this commit dotansimha@ca4ab0d , it seems to work and it does not fail on the time issue.

I'm trying to use minitrace::flush but at the moment I only get the root Span reported and all the rest are ignored 🤔
My current structure is like that:

- root (Span::root)
	- `async fn` with #[trace]
	- `async fn` with #[trace] 
		- ... several `async fn`, some with `#[trace]` and some with `in_span`
	- sync `fn` with #[trace]

The macro seems to set #[trace] to use in_span, and i think it should be fine?

image

EDIT: Also using in_span explicitly doesn't work for the child Spans. I only get the root reported.

🤔

@andylokandy
Copy link
Collaborator Author

did you set_local_parent()? and drop all spans before flush?

@dotansimha
Copy link
Contributor

did you set_local_parent()? and drop all spans before flush?

oops, I missed that 🤦 I added it now and it seems to work! 🎉

So I think the only fix needed for PR is dotansimha@ca4ab0d

@andylokandy andylokandy merged commit 34fccc7 into tikv:master Feb 5, 2024
9 checks passed
@dotansimha
Copy link
Contributor

@andylokandy can we get this released to Crates? 👀

Thank you

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.

LocalSpan::enter_with_parent missing?
4 participants