feat: add LocalSpans::to_span_records()

[andylokandy]

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

Closes #184

[andylokandy]

@dotansimha Does it work for your project?

[dotansimha]

@andylokandy thank you for this PR!

How would a usage of this configuration look like? I assume it might still need threads and a global collector that's being initialized? 🤔

I just created this PR: #186 , but it might be a too big of a change?

[andylokandy]

Here is the example:

minitrace-rust/minitrace/src/local/local_collector.rs

Lines 188 to 224 in 0337d43

	impl LocalSpans {
	/// Converts the `LocalSpans` to `SpanRecord`s.
	///
	/// The converted spans will appear as if they were collected within the given parent context.
	/// The parent of the top local span is set to the given parent.
	///
	/// This function is particularly useful when you want to manually collect the span records
	/// without involving the global collector. This function does not require that the global
	/// collector is set up by [`set_reporter()`].
	///
	/// # Example
	///
	/// ```rust
	/// use minitrace::local::LocalCollector;
	/// use minitrace::local::LocalSpans;
	/// use minitrace::prelude::*;
	///
	/// // Collect local spans manually without a parent
	/// let collector = LocalCollector::start();
	/// let span = LocalSpan::enter_with_local_parent("a child span");
	/// drop(span);
	///
	/// // Collect local spans into a LocalSpans instance
	/// let local_spans: LocalSpans = collector.collect();
	///
	/// // Convert LocalSpans to SpanRecords with a given parent context
	/// let parent_context = SpanContext::random();
	/// let span_records = local_spans.to_span_records(parent_context);
	///
	/// // Now you can manually handle the span records
	/// for record in span_records {
	/// println!("{:?}", record);
	/// }
	/// ```
	///
	/// [`set_reporter()`]: crate::set_reporter
	pub fn to_span_records(&self, parent: SpanContext) -> Vec<SpanRecord> {

[coveralls]

Pull Request Test Coverage Report for Build 7637141736

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.2%) to 80.9%

---

Totals
Change from base Build 6945473252:	0.2%
Covered Lines:	1690
Relevant Lines:	2089

---

💛 - Coveralls

[dotansimha]

Thanks @andylokandy !

I gave it a spin and it seems to fail at runtime: time not implemented on this platform (on WASM runtime).

All I did so far it to add minitrace with features = ["enable"] and did the following at startup:

  let collector = LocalCollector::start();
  let result = run_flow(req, env).await;
  let local_spans = collector.collect();
  let parent_context = SpanContext::random();
  let span_records = local_spans.to_span_records(parent_context); // ❗ 

I'm pretty sure this originates in to_span_records, because without calling it, it seems to work fine. I printed local_spans and it seems to correctly contain the spans 🎉

[dotansimha]

One more thing that I noticed, and not sure how to confirm: seems like only spans created with #[trace] are collected with this kind of collector. Spans attached with in_span are not collected? 🤔

EDIT: Seems like using enter_with_local_parent works. I can live with that 👍

[andylokandy]

I gave it a spin and it seems to fail at runtime: time not implemented on this platform (on WASM runtime).

Could you use SystemTime::now() on wasm target? You may also try wasm32-wasi target. If none of them works, you need to enable #[feature(wasm_syscall)]

One more thing that I noticed, and not sure how to confirm: seems like only spans created with #[trace] are collected with this kind of collector. Spans attached with in_span are not collected?

In a LocalCollector set up, Span will become noop, since it's a Threading-thing.

[dotansimha]

Could you use SystemTime::now() on wasm target? You may also try wasm32-wasi target.

I'm not able to use wasm32-wasi because of a limitation for the runtime (CloudFlare Worker), it's only using wasm32-unknown-unknown (related: cloudflare/workers-rs#435).

And SystemTime::now() is not supported on this target, see rust-lang/rust#48564 (comment)

[andylokandy]

Let's move the wasm discussion to #187

[dotansimha]

I can confirm that with the fix in tikv/minstant#34 , this PR works great!