-
Notifications
You must be signed in to change notification settings - Fork 21
feat(telemetry)!: make telemetry worker wasm-compatible for the TraceExporter #2172
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
Changes from all commits
a9251df
800823d
fa53c71
2f63fe1
cb8ddeb
c5b608f
05b4735
74adfb8
9b6dbd9
b7acc91
96930b6
d7aa577
b3aef55
f6cecb9
cf5b876
7846115
ef5395e
bc57217
19987a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,8 @@ use crate::{ErrorKind, SigInfo}; | |||||||||||||||||||||||||
| use super::{CrashInfo, Metadata, TARGET_TRIPLE}; | ||||||||||||||||||||||||||
| use anyhow::Context; | ||||||||||||||||||||||||||
| use chrono::{DateTime, Utc}; | ||||||||||||||||||||||||||
| use libdd_capabilities::HttpClientCapability; | ||||||||||||||||||||||||||
| use libdd_capabilities_impl::NativeCapabilities; | ||||||||||||||||||||||||||
| use libdd_common::Endpoint; | ||||||||||||||||||||||||||
| use libdd_telemetry::{ | ||||||||||||||||||||||||||
| build_host, | ||||||||||||||||||||||||||
|
|
@@ -355,9 +357,9 @@ impl TelemetryCrashUploader { | |||||||||||||||||||||||||
| self.send_telemetry_payload(&payload).await | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// Helper to perform actual HTTP (or file) submission via configured telemetry client | ||||||||||||||||||||||||||
| /// Helper to perform actual HTTP submission via the native HTTP capability. | ||||||||||||||||||||||||||
| async fn send_telemetry_payload(&self, payload: &data::Telemetry<'_>) -> anyhow::Result<()> { | ||||||||||||||||||||||||||
| let client = libdd_telemetry::worker::http_client::from_config(&self.cfg); | ||||||||||||||||||||||||||
| let client = NativeCapabilities::new_client(); | ||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we properly propagate configuration from |
||||||||||||||||||||||||||
| let req = request_builder(&self.cfg)? | ||||||||||||||||||||||||||
| .method(http::Method::POST) | ||||||||||||||||||||||||||
| .header( | ||||||||||||||||||||||||||
|
|
@@ -372,19 +374,20 @@ impl TelemetryCrashUploader { | |||||||||||||||||||||||||
| libdd_telemetry::worker::http_client::header::REQUEST_TYPE, | ||||||||||||||||||||||||||
| "logs", | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| .body(serde_json::to_string(&payload)?.into())?; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| tokio::time::timeout( | ||||||||||||||||||||||||||
| core::time::Duration::from_millis({ | ||||||||||||||||||||||||||
| if let Some(endp) = self.cfg.endpoint() { | ||||||||||||||||||||||||||
| endp.timeout_ms | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| Endpoint::DEFAULT_TIMEOUT | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||
| client.request(req), | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| .await??; | ||||||||||||||||||||||||||
| .body(libdd_capabilities::Bytes::from(serde_json::to_vec( | ||||||||||||||||||||||||||
| &payload, | ||||||||||||||||||||||||||
| )?))?; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| let timeout = core::time::Duration::from_millis({ | ||||||||||||||||||||||||||
| if let Some(endp) = self.cfg.endpoint() { | ||||||||||||||||||||||||||
| endp.timeout_ms | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| Endpoint::DEFAULT_TIMEOUT | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
|
Comment on lines
+381
to
+387
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think I'd make another PR for styling like that, but it definitely would have been good |
||||||||||||||||||||||||||
| tokio::time::timeout(timeout, client.request(req)) | ||||||||||||||||||||||||||
| .await | ||||||||||||||||||||||||||
| .map_err(|_| anyhow::anyhow!("Telemetry crash report timed out"))??; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Ok(()) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ serde_json = "1.0.127" | |
| bytes = "1.11.1" | ||
| sha2 = "0.10" | ||
| either = "1.13.0" | ||
| futures = { version = "0.3", default-features = false, features = ["alloc"] } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we use this for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was not usable with wasm because it spawned threads |
||
| tokio = { version = "1.23", features = [ | ||
| "rt", | ||
| "sync", | ||
|
|
@@ -46,6 +47,7 @@ libdd-tinybytes = { version = "1.1.1", path = "../libdd-tinybytes", features = [ | |
| "bytes_string", | ||
| "serialization", | ||
| ] } | ||
| web-time = "1" | ||
|
|
||
| [target.'cfg(not(target_arch = "wasm32"))'.dependencies] | ||
| tokio = { version = "1.23", features = ["time", "test-util"], default-features = false } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should not be cloned, but just shared as a reference instead. This probably duplicate the
OnceLockand thus always create a client from scratchThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an Arc, so it shouldn't