Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit f9de9d9

Browse files
calebschoeppcalebschoepp
committedAug 29, 2024··
host-guest-host test and wire ObserveContext into outbound http
Signed-off-by: Caleb Schoepp <[email protected]>
1 parent f78566b commit f9de9d9

File tree

13 files changed

+133
-167
lines changed

13 files changed

+133
-167
lines changed
 

‎Cargo.lock

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/factor-observe/Cargo.toml

+1-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ once_cell = "1"
1414
opentelemetry = { version = "0.22.0", features = [ "metrics", "trace"] }
1515
opentelemetry_sdk = { version = "0.22.1", features = ["rt-tokio"] }
1616
opentelemetry-otlp = { version = "0.15.0", default-features=false, features = ["http-proto", "trace", "http", "reqwest-client", "metrics", "grpc-tonic"] }
17-
pin-project-lite = "0.2"
1817
serde = "1.0.188"
1918
spin-app = { path = "../app" }
2019
spin-core = { path = "../core" }
@@ -35,4 +34,4 @@ toml = "0.5"
3534
[lints]
3635
workspace = true
3736

38-
# TODO(Caleb): Cleanup these dependencies
37+
# TODO(Caleb): Cleanup these dependencies, use workspace, remove not needed

‎crates/factor-observe/src/future.rs

-80
This file was deleted.

‎crates/factor-observe/src/host.rs

+1
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ impl traces::HostSpan for InstanceState {
123123

124124
fn drop(&mut self, _resource: Resource<WitSpan>) -> Result<()> {
125125
// TODO(Caleb): What do we want the dropping behavior to be?
126+
// TODO(Caleb): How is the drop semantics test passing?
126127
Ok(())
127128
}
128129
}

‎crates/factor-observe/src/lib.rs

+30-81
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
pub mod future;
21
mod host;
32

43
use std::sync::{Arc, RwLock};
54

6-
use future::ObserveContext;
75
use indexmap::IndexMap;
6+
use opentelemetry::{global::ObjectSafeSpan, trace::TraceContextExt, Context};
87
use spin_factors::{Factor, SelfInstanceBuilder};
8+
use tracing_opentelemetry::OpenTelemetrySpanExt;
99

1010
#[derive(Default)]
1111
pub struct ObserveFactor {}
@@ -57,29 +57,6 @@ pub struct InstanceState {
5757
impl SelfInstanceBuilder for InstanceState {}
5858

5959
impl InstanceState {
60-
// /// Close the span associated with the given resource and optionally drop the resource
61-
// /// from the table. Additionally close any other active spans that are more recent on the stack
62-
// /// in reverse order.
63-
// ///
64-
// /// Exiting any spans that were already closed will not cause this to error.
65-
// fn safely_close(&mut self, resource_id: u32, drop_resource: bool) {
66-
// let mut state: std::sync::RwLockWriteGuard<State> = self.state.write().unwrap();
67-
68-
// if let Some(index) = state
69-
// .active_spans
70-
// .iter()
71-
// .rposition(|(_, id)| *id == resource_id)
72-
// {
73-
// state.close_from_back_to(index);
74-
// } else {
75-
// tracing::debug!("found no active spans to close")
76-
// }
77-
78-
// if drop_resource {
79-
// state.guest_spans.remove(resource_id).unwrap();
80-
// }
81-
// }
82-
8360
pub fn get_observe_context(&self) -> ObserveContext {
8461
ObserveContext {
8562
state: self.state.clone(),
@@ -100,65 +77,37 @@ pub(crate) struct State {
10077
pub active_spans: IndexMap<String, u32>,
10178
}
10279

103-
// impl State {
104-
// /// Close all active spans from the top of the stack to the given index. Closing entails exiting
105-
// /// the inner [tracing] span and removing it from the active spans stack.
106-
// pub(crate) fn close_from_back_to(&mut self, index: usize) {
107-
// self.active_spans
108-
// .split_off(index)
109-
// .iter()
110-
// .rev()
111-
// .for_each(|(_, id)| {
112-
// if let Some(guest_span) = self.guest_spans.get(*id) {
113-
// guest_span.exit();
114-
// } else {
115-
// tracing::debug!("active_span {id:?} already removed from resource table");
116-
// }
117-
// });
118-
// }
119-
120-
// /// Enter the inner [tracing] span for all active spans.
121-
// pub(crate) fn enter_all(&self) {
122-
// for (_, guest_span_id) in self.active_spans.iter() {
123-
// if let Some(span_resource) = self.guest_spans.get(*guest_span_id) {
124-
// span_resource.enter();
125-
// } else {
126-
// tracing::debug!("guest span already dropped")
127-
// }
128-
// }
129-
// }
130-
131-
// /// Exit the inner [tracing] span for all active spans.
132-
// pub(crate) fn exit_all(&self) {
133-
// for (_, guest_span_id) in self.active_spans.iter().rev() {
134-
// if let Some(span_resource) = self.guest_spans.get(*guest_span_id) {
135-
// span_resource.exit();
136-
// } else {
137-
// tracing::debug!("guest span already dropped")
138-
// }
139-
// }
140-
// }
141-
// }
142-
14380
/// The WIT resource Span. Effectively wraps an [opentelemetry_sdk::trace::Span].
14481
pub struct GuestSpan {
14582
/// The [opentelemetry_sdk::trace::Span] we use to do the actual tracing work.
14683
pub inner: opentelemetry_sdk::trace::Span,
14784
}
14885

149-
// // Note: We use tracing enter instead of Entered because Entered is not Send
150-
// impl GuestSpan {
151-
// /// Enter the inner [tracing] span.
152-
// pub fn enter(&self) {
153-
// self.inner.with_subscriber(|(id, dispatch)| {
154-
// dispatch.enter(id);
155-
// });
156-
// }
157-
158-
// /// Exits the inner [tracing] span.
159-
// pub fn exit(&self) {
160-
// self.inner.with_subscriber(|(id, dispatch)| {
161-
// dispatch.exit(id);
162-
// });
163-
// }
164-
// }
86+
pub struct ObserveContext {
87+
pub(crate) state: Arc<RwLock<State>>,
88+
}
89+
90+
impl ObserveContext {
91+
/// TODO comment
92+
pub fn oh_dear_i_better_get_renamed(&self) {
93+
// TODO: Move this duplicate logic into its own impl
94+
let state = self.state.read().unwrap();
95+
if state.active_spans.is_empty() {
96+
return;
97+
}
98+
99+
let parent_context = Context::new().with_remote_span_context(
100+
state
101+
.guest_spans
102+
.get(*state.active_spans.last().unwrap().1)
103+
.unwrap()
104+
.inner
105+
.span_context()
106+
.clone(),
107+
);
108+
tracing::Span::current().set_parent(parent_context);
109+
}
110+
}
111+
112+
// TODO(Caleb): Reorder things
113+
// TODO(Caleb): Make otel a workspace dependency

‎crates/factor-outbound-http/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ http-body-util = "0.1"
1111
hyper = "1.4.1"
1212
reqwest = { version = "0.11", features = ["gzip"] }
1313
rustls = { version = "0.23", default-features = false, features = ["ring", "std"] }
14+
spin-factor-observe = { path = "../factor-observe" }
1415
spin-factor-outbound-networking = { path = "../factor-outbound-networking" }
1516
spin-factors = { path = "../factors" }
1617
spin-telemetry = { path = "../telemetry" }

‎crates/factor-outbound-http/src/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use http::{
1010
uri::{Authority, Parts, PathAndQuery, Scheme},
1111
HeaderValue, Uri,
1212
};
13+
use spin_factor_observe::{ObserveContext, ObserveFactor};
1314
use spin_factor_outbound_networking::{
1415
ComponentTlsConfigs, OutboundAllowedHosts, OutboundNetworkingFactor,
1516
};
@@ -65,13 +66,15 @@ impl Factor for OutboundHttpFactor {
6566
let outbound_networking = builders.get_mut::<OutboundNetworkingFactor>()?;
6667
let allowed_hosts = outbound_networking.allowed_hosts();
6768
let component_tls_configs = outbound_networking.component_tls_configs().clone();
69+
let observe_context = builders.get_mut::<ObserveFactor>()?.get_observe_context();
6870
Ok(InstanceState {
6971
wasi_http_ctx: WasiHttpCtx::new(),
7072
allowed_hosts,
7173
component_tls_configs,
7274
self_request_origin: None,
7375
request_interceptor: None,
7476
spin_http_client: None,
77+
observe_context,
7578
})
7679
}
7780
}
@@ -84,6 +87,7 @@ pub struct InstanceState {
8487
request_interceptor: Option<Box<dyn OutboundHttpInterceptor>>,
8588
// Connection-pooling client for 'fermyon:spin/http' interface
8689
spin_http_client: Option<reqwest::Client>,
90+
observe_context: ObserveContext,
8791
}
8892

8993
impl InstanceState {

‎crates/factor-outbound-http/src/spin.rs

+2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ impl spin_http::Host for crate::InstanceState {
1313
fields(otel.kind = "client", url.full = Empty, http.request.method = Empty,
1414
http.response.status_code = Empty, otel.name = Empty, server.address = Empty, server.port = Empty))]
1515
async fn send_request(&mut self, req: Request) -> Result<Response, HttpError> {
16+
self.observe_context.oh_dear_i_better_get_renamed();
17+
1618
let span = Span::current();
1719
record_request_fields(&span, &req);
1820

‎crates/factor-outbound-http/src/wasi.rs

+2
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ impl<'a> WasiHttpView for WasiHttpImplInner<'a> {
8686
mut request: Request<wasmtime_wasi_http::body::HyperOutgoingBody>,
8787
mut config: wasmtime_wasi_http::types::OutgoingRequestConfig,
8888
) -> wasmtime_wasi_http::HttpResult<wasmtime_wasi_http::types::HostFutureIncomingResponse> {
89+
self.state.observe_context.oh_dear_i_better_get_renamed();
90+
8991
// wasmtime-wasi-http fills in scheme and authority for relative URLs
9092
// (e.g. https://:443/<path>), which makes them hard to reason about.
9193
// Undo that here.

‎crates/trigger/src/factors.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use spin_runtime_config::TomlRuntimeConfigSource;
1818

1919
#[derive(RuntimeFactors)]
2020
pub struct TriggerFactors {
21+
pub observe: ObserveFactor,
2122
pub wasi: WasiFactor,
2223
pub variables: VariablesFactor,
2324
pub key_value: KeyValueFactor,
@@ -29,7 +30,6 @@ pub struct TriggerFactors {
2930
pub pg: OutboundPgFactor,
3031
pub mysql: OutboundMysqlFactor,
3132
pub llm: LlmFactor,
32-
pub observe: ObserveFactor,
3333
}
3434

3535
impl TriggerFactors {
@@ -42,6 +42,7 @@ impl TriggerFactors {
4242
use_gpu: bool,
4343
) -> anyhow::Result<Self> {
4444
Ok(Self {
45+
observe: ObserveFactor::new(),
4546
wasi: wasi_factor(working_dir, allow_transient_writes),
4647
variables: VariablesFactor::default(),
4748
key_value: KeyValueFactor::new(default_key_value_label_resolver),
@@ -56,7 +57,6 @@ impl TriggerFactors {
5657
spin_factor_llm::spin::default_engine_creator(state_dir, use_gpu)
5758
.context("failed to configure LLM factor")?,
5859
),
59-
observe: ObserveFactor::new(),
6060
})
6161
}
6262
}

‎tests/integration.rs

+72
Original file line numberDiff line numberDiff line change
@@ -434,13 +434,85 @@ mod integration_tests {
434434
Ok(())
435435
}
436436

437+
#[tokio::test]
438+
async fn wasi_observe_host_guest_host() -> anyhow::Result<()> {
439+
let collector = FakeCollectorServer::start()
440+
.await
441+
.expect("fake collector server should start");
442+
let collector_endpoint = collector.endpoint().clone();
443+
444+
tokio::task::spawn_blocking(|| {
445+
run_test_inited(
446+
"wasi-observe-tracing",
447+
SpinConfig {
448+
binary_path: spin_binary(),
449+
spin_up_args: Vec::new(),
450+
app_type: SpinAppType::Http,
451+
},
452+
ServicesConfig::none(),
453+
|env| {
454+
env.set_env_var("OTEL_EXPORTER_OTLP_ENDPOINT", collector_endpoint);
455+
env.set_env_var("OTEL_EXPORTER_OTLP_TRACES_PROTOCOL", "grpc");
456+
env.set_env_var("OTEL_BSP_SCHEDULE_DELAY", "5");
457+
Ok(())
458+
},
459+
move |env| {
460+
let spin = env.runtime_mut();
461+
assert_spin_request(
462+
spin,
463+
Request::new(Method::Get, "/host-guest-host"),
464+
Response::new(200),
465+
)?;
466+
467+
let mut spans: Vec<ExportedSpan>;
468+
assert_eventually!(
469+
{
470+
spans = collector.exported_spans();
471+
!spans.is_empty()
472+
},
473+
5
474+
);
475+
476+
assert_eq!(spans.len(), 4);
477+
478+
assert!(spans
479+
.iter()
480+
.map(|s| s.trace_id.clone())
481+
.all(|t| t == spans[0].trace_id));
482+
483+
let exec_component_span = spans
484+
.iter()
485+
.find(|s| s.name == "execute_wasm_component wasi-observe-tracing")
486+
.expect("'execute_wasm_component wasi-observe-tracing' span should exist");
487+
let guest_span = spans
488+
.iter()
489+
.find(|s| s.name == "guest")
490+
.expect("'guest' span should exist");
491+
let get_span = spans
492+
.iter()
493+
.find(|s| s.name == "GET")
494+
.expect("'GET' span should exist");
495+
496+
assert_eq!(guest_span.parent_span_id, exec_component_span.span_id);
497+
assert_eq!(get_span.parent_span_id, guest_span.span_id);
498+
499+
Ok(())
500+
},
501+
)
502+
})
503+
.await??;
504+
505+
Ok(())
506+
}
507+
437508
// TODO: wasi_observe_set_event
438509
// TODO: wasi_observe_update_name
439510
// TODO: wasi_observe_set_link
440511
// TODO: wasi_observe_host_guest_host
441512
// TODO: semantics of closing a parent doesn't close child
442513
// TODO: inbound trace propagation
443514
// TODO: outbound trace propagation
515+
// TODO: Weird edge cases where a span is closed before we try to load it implicitly as parent or as observecontext thing.
444516

445517
#[test]
446518
/// Test dynamic environment variables

‎tests/test-components/components/wasi-observe-tracing/src/lib.rs

+15-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ wit_bindgen::generate!({
55
});
66

77
use spin_sdk::{
8-
http::{Params, Request, Response, Router},
8+
http::{Method, Params, Request, Response, Router},
99
http_component,
1010
};
1111
use wasi::observe::traces::{KeyValue, Span, Value};
@@ -16,6 +16,7 @@ fn handle(req: http::Request<()>) -> Response {
1616
router.get("/nested-spans", nested_spans);
1717
router.get("/drop-semantics", drop_semantics);
1818
router.get("/setting-attributes", setting_attributes);
19+
router.get_async("/host-guest-host", host_guest_host);
1920
router.handle(req)
2021
}
2122

@@ -56,3 +57,16 @@ fn setting_attributes(_req: Request, _params: Params) -> Response {
5657
span.end();
5758
Response::new(200, "")
5859
}
60+
61+
async fn host_guest_host(_req: Request, _params: Params) -> Response {
62+
let span = Span::start("guest");
63+
64+
let req = Request::builder()
65+
.method(Method::Get)
66+
.uri("https://asdf.com")
67+
.build();
68+
let _res: Response = spin_sdk::http::send(req).await.unwrap();
69+
span.end();
70+
71+
Response::new(200, "")
72+
}

‎tests/testcases/wasi-observe-tracing/spin.toml

+2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ component = "wasi-observe-tracing"
1212

1313
[component.wasi-observe-tracing]
1414
source = "%{source=wasi-observe-tracing}"
15+
key_value_stores = ["default"]
16+
allowed_outbound_hosts = ["http://self", "https://asdf.com"]
1517
[component.wasi-observe-tracing.build]
1618
command = "cargo build --target wasm32-wasi --release"
1719
watch = ["src/**/*.rs", "Cargo.toml"]

0 commit comments

Comments
 (0)
Please sign in to comment.