hellovai/tracing #1350
Draft
hellovai/tracing #1350
Conversation
This file contains 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
Contributor
hellovai
commented
Jan 20, 2025
hellovai
commented
- tracing impl for context
- add new tracing client impl
- stash everything
- compile again
- move tracing types around
- one more
- Merge latest
- tracing WIP
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update enhances the project's tracing capabilities by introducing a new tracing module, adding necessary dependencies, and integrating these features across various components. Key changes include:
Changes
Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts belowEmoji Descriptions:
Interact with the Bot:
|
Comment on lines
+13
to
+31
| // THESE ARE NOT CLONEABLE!! | ||
| pub struct LogEvent { | ||
| /* | ||
| * (span_id, content_span_id) is a unique identifier for a log event | ||
| * The query (span_id, *) gets all logs for a function call | ||
| */ | ||
|
|
||
| pub span_id: FunctionId, | ||
| pub content_span_id: ContentId, | ||
|
|
||
| // The chain of spans that lead to this log event | ||
| // Includes span_id at the last position (content_span_id is not included) | ||
| pub span_chain: Vec<FunctionId>, | ||
|
|
||
| // The timestamp of the log | ||
| pub timestamp: web_time::Instant, | ||
| // The content of the log | ||
| pub content: LogEventContent, | ||
| } |
There was a problem hiding this comment.
The LogEvent struct is missing Clone, PartialEq, and Hash traits which are needed for proper event tracking and comparison. Add these trait derivations.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| // THESE ARE NOT CLONEABLE!! | |
| pub struct LogEvent { | |
| /* | |
| * (span_id, content_span_id) is a unique identifier for a log event | |
| * The query (span_id, *) gets all logs for a function call | |
| */ | |
| pub span_id: FunctionId, | |
| pub content_span_id: ContentId, | |
| // The chain of spans that lead to this log event | |
| // Includes span_id at the last position (content_span_id is not included) | |
| pub span_chain: Vec<FunctionId>, | |
| // The timestamp of the log | |
| pub timestamp: web_time::Instant, | |
| // The content of the log | |
| pub content: LogEventContent, | |
| } | |
| #[derive(Clone, PartialEq, Hash)] | |
| pub struct LogEvent { | |
| /* | |
| * (span_id, content_span_id) is a unique identifier for a log event | |
| * The query (span_id, *) gets all logs for a function call | |
| */ | |
| pub span_id: FunctionId, | |
| pub content_span_id: ContentId, | |
| // The chain of spans that lead to this log event | |
| // Includes span_id at the last position (content_span_id is not included) | |
| pub span_chain: Vec<FunctionId>, | |
| // The timestamp of the log | |
| pub timestamp: web_time::Instant, | |
| // The content of the log | |
| pub content: LogEventContent, | |
| } |
Comment on lines
+38
to
+39
| let batch = TraceEventBatch { events }; | ||
| let body = serde_json::to_string(&batch).unwrap(); |
There was a problem hiding this comment.
Using unwrap() on serde_json::to_string() can panic if serialization fails. Should use proper error handling with ? operator.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| let batch = TraceEventBatch { events }; | |
| let body = serde_json::to_string(&batch).unwrap(); | |
| let batch = TraceEventBatch { events }; | |
| let body = serde_json::to_string(&batch)? |
Comment on lines
+83
to
+102
| let client = reqwest::Client::new(); | ||
| let start = Instant::now(); | ||
|
|
||
| let request_body = GetSignedUrlRequest { | ||
| bucket: bucket.to_string(), | ||
| key: format!("{}/{}.json", key_prefix, uuid::Uuid::new_v4()), | ||
| }; | ||
|
|
||
| eprintln!("Making POST request to URL: {}", lambda_url); | ||
| eprintln!( | ||
| "Request body: {}", | ||
| serde_json::to_string_pretty(&request_body)? | ||
| ); | ||
|
|
||
| let response = client | ||
| .post(lambda_url) | ||
| .json(&request_body) | ||
| .send() | ||
| .await | ||
| .context("Failed to call lambda function")?; |
There was a problem hiding this comment.
Creating a new reqwest::Client for each request is inefficient. The client should be reused across requests by making it static or passing it as a parameter.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| let client = reqwest::Client::new(); | |
| let start = Instant::now(); | |
| let request_body = GetSignedUrlRequest { | |
| bucket: bucket.to_string(), | |
| key: format!("{}/{}.json", key_prefix, uuid::Uuid::new_v4()), | |
| }; | |
| eprintln!("Making POST request to URL: {}", lambda_url); | |
| eprintln!( | |
| "Request body: {}", | |
| serde_json::to_string_pretty(&request_body)? | |
| ); | |
| let response = client | |
| .post(lambda_url) | |
| .json(&request_body) | |
| .send() | |
| .await | |
| .context("Failed to call lambda function")?; | |
| let client = &CLIENT; | |
| let start = Instant::now(); | |
| let request_body = GetSignedUrlRequest { | |
| bucket: bucket.to_string(), | |
| key: format!("{}/{}.json", key_prefix, uuid::Uuid::new_v4()), | |
| }; | |
| eprintln!("Making POST request to URL: {}", lambda_url); | |
| eprintln!( | |
| "Request body: {}", | |
| serde_json::to_string_pretty(&request_body)? | |
| ); | |
| let response = client | |
| .post(lambda_url) | |
| .json(&request_body) | |
| .send() | |
| .await | |
| .context("Failed to call lambda function")? |
Comment on lines
+9
to
+16
| let localset = tokio::task::LocalSet::new(); | ||
| localset.spawn_local(async move { | ||
| if let Err(e) = run().await { | ||
| tracing::error!("server error: {}", e); | ||
| } | ||
| }); | ||
|
|
||
| println!("shutting down server"); |
There was a problem hiding this comment.
The localset is created but never executed with .await or run, causing the server to exit immediately after spawning the task. Fix by adding localset.await; after spawning.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| let localset = tokio::task::LocalSet::new(); | |
| localset.spawn_local(async move { | |
| if let Err(e) = run().await { | |
| tracing::error!("server error: {}", e); | |
| } | |
| }); | |
| println!("shutting down server"); | |
| let localset = tokio::task::LocalSet::new(); | |
| localset.spawn_local(async move { | |
| if let Err(e) = run().await { | |
| tracing::error!("server error: {}", e); | |
| } | |
| }); | |
| localset.await; | |
| println!("shutting down server"); |
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.