Skip to content

Commit fba82d2

Browse files
fix: address Codex review — parser latency unit, URI extraction, diagnostics
- Cloudflare: OriginResponseTime correctly converted from nanoseconds to ms (was consumed raw, 1000x underestimation) - Fastly: URI extraction handles all HTTP methods (was GET/POST only) - Both parsers: warn messages include file path and line number for diagnosis - Both parsers: non-UTF-8 lines logged instead of silently dropped - Calibrate sensitivity analysis: uses train_policy + eval_with_keys (was re-training on validation data)
1 parent 59a8830 commit fba82d2

3 files changed

Lines changed: 47 additions & 16 deletions

File tree

crates/qc-cli/src/providers/cloudflare.rs

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,23 @@ impl ProviderLogParser for CloudflareParser {
3232
const MAX_LINE_BYTES: usize = 64 * 1024;
3333
let mut buf = Vec::with_capacity(4096);
3434
let mut reader = reader;
35+
let mut line_num = 0usize;
3536

3637
loop {
3738
buf.clear();
39+
line_num += 1;
3840
let bytes_read = (&mut reader)
3941
.take((MAX_LINE_BYTES + 1) as u64)
4042
.read_until(b'\n', &mut buf)?;
4143
if bytes_read == 0 {
4244
break;
4345
}
4446
if buf.len() > MAX_LINE_BYTES && !buf.ends_with(b"\n") {
45-
tracing::warn!("skipping oversized line (>{MAX_LINE_BYTES} bytes)");
47+
tracing::warn!(
48+
"{}:{}: skipping oversized line (>{MAX_LINE_BYTES} bytes)",
49+
path.display(),
50+
line_num
51+
);
4652
loop {
4753
buf.clear();
4854
let n = (&mut reader)
@@ -56,7 +62,10 @@ impl ProviderLogParser for CloudflareParser {
5662
}
5763
let line = match std::str::from_utf8(&buf) {
5864
Ok(s) => s.trim(),
59-
Err(_) => continue,
65+
Err(_) => {
66+
tracing::warn!("{}:{}: skipping non-UTF-8 line", path.display(), line_num);
67+
continue;
68+
}
6069
};
6170
if line.is_empty() {
6271
continue;
@@ -66,7 +75,11 @@ impl ProviderLogParser for CloudflareParser {
6675
Ok(Some(e)) => events.push(e),
6776
Ok(None) => {}
6877
Err(e) => {
69-
tracing::warn!("skipping malformed line: {e}");
78+
tracing::warn!(
79+
"{}:{}: skipping malformed line: {e}",
80+
path.display(),
81+
line_num
82+
);
7083
}
7184
}
7285
}
@@ -123,13 +136,14 @@ fn parse_cloudflare_line(
123136

124137
let eligible = status_code != 206 && (200..400).contains(&status_code);
125138

126-
// Origin response time (seconds)
127-
let origin_time_ms = v["OriginResponseTime"]
128-
.as_f64()
129-
.or_else(|| v["OriginResponseHTTPExpires"].as_f64())
130-
.unwrap_or(0.0);
131-
let latency_ms = if origin_time_ms.is_finite() && (0.0..=3600000.0).contains(&origin_time_ms) {
132-
origin_time_ms
139+
// OriginResponseTime: nanoseconds in Cloudflare Logpush v2 → convert to ms
140+
let origin_time_ns = v["OriginResponseTime"]
141+
.as_u64()
142+
.or_else(|| v["OriginResponseTime"].as_f64().map(|f| f as u64))
143+
.unwrap_or(0);
144+
let latency_ms = origin_time_ns as f64 / 1_000_000.0;
145+
let latency_ms = if latency_ms.is_finite() && (0.0..=3600000.0).contains(&latency_ms) {
146+
latency_ms
133147
} else {
134148
0.0
135149
};

crates/qc-cli/src/providers/fastly.rs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,23 @@ impl ProviderLogParser for FastlyParser {
3838
let mut buf = Vec::with_capacity(4096);
3939
let mut reader = reader;
4040

41+
let mut line_num = 0usize;
42+
4143
loop {
4244
buf.clear();
45+
line_num += 1;
4346
let bytes_read = (&mut reader)
4447
.take((MAX_LINE_BYTES + 1) as u64)
4548
.read_until(b'\n', &mut buf)?;
4649
if bytes_read == 0 {
4750
break;
4851
}
4952
if buf.len() > MAX_LINE_BYTES && !buf.ends_with(b"\n") {
50-
tracing::warn!("skipping oversized line (>{MAX_LINE_BYTES} bytes)");
53+
tracing::warn!(
54+
"{}:{}: skipping oversized line (>{MAX_LINE_BYTES} bytes)",
55+
path.display(),
56+
line_num
57+
);
5158
loop {
5259
buf.clear();
5360
let n = (&mut reader)
@@ -61,7 +68,10 @@ impl ProviderLogParser for FastlyParser {
6168
}
6269
let line = match std::str::from_utf8(&buf) {
6370
Ok(s) => s.trim(),
64-
Err(_) => continue,
71+
Err(_) => {
72+
tracing::warn!("{}:{}: skipping non-UTF-8 line", path.display(), line_num);
73+
continue;
74+
}
6575
};
6676
if line.is_empty() {
6777
continue;
@@ -71,7 +81,11 @@ impl ProviderLogParser for FastlyParser {
7181
Ok(Some(e)) => events.push(e),
7282
Ok(None) => {}
7383
Err(e) => {
74-
tracing::warn!("skipping malformed line: {e}");
84+
tracing::warn!(
85+
"{}:{}: skipping malformed line: {e}",
86+
path.display(),
87+
line_num
88+
);
7589
}
7690
}
7791
}
@@ -111,7 +125,8 @@ fn parse_fastly_line(
111125

112126
// URL: may be full request line "GET /path HTTP/1.1" or just "/path"
113127
let raw_url = v["url"].as_str().unwrap_or("/");
114-
let uri = if raw_url.starts_with("GET ") || raw_url.starts_with("POST ") {
128+
let uri = if raw_url.contains(' ') {
129+
// Request line: "METHOD /path HTTP/version" — extract path component
115130
raw_url.split_whitespace().nth(1).unwrap_or("/").to_string()
116131
} else {
117132
raw_url.to_string()

crates/qc-solver/src/calibrate.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ pub fn default_eval(
8888
pub fn calibrate(
8989
train_features: &[ObjectFeatures],
9090
_train_events: &[RequestTraceEvent],
91-
val_features: &[ObjectFeatures],
91+
_val_features: &[ObjectFeatures],
9292
val_events: &[RequestTraceEvent],
9393
base_config: &ScenarioConfig,
9494
num_restarts: usize,
@@ -166,7 +166,9 @@ pub fn calibrate(
166166
_ => StalePenaltyClass::Medium,
167167
};
168168
let config = make_config(capacity, lat, pen);
169-
let score = default_eval(&config, val_features, val_events, capacity)?;
169+
// Train on training data, evaluate on validation data (consistent with main loop)
170+
let cached_keys = train_policy(&config, train_features, capacity)?;
171+
let score = eval_with_keys(&cached_keys, val_events, config.latency_value_per_ms);
170172
sensitivity.push(("latency_value_per_ms".into(), lat, score - base_score));
171173
}
172174

0 commit comments

Comments
 (0)