Skip to content

Commit 3bca1bb

Browse files
authored
Adds Trace and Summary to CLI instrumented stores (#18064)
- Adds the ability for a user to choose a summary only output for an instrumented object store when using the CLI - The existing "enabled" setting that displays both a summary and a detailed usage for each object store call has been renamed to `Trace` to improve clarity - Adds additional test cases for summary only and modifies existing tests to use trace - Updates user guide docs to reflect the CLI flag and command line changes
1 parent 4153adf commit 3bca1bb

File tree

7 files changed

+75
-31
lines changed

7 files changed

+75
-31
lines changed

datafusion-cli/src/command.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ impl Command {
128128
let profile_mode = mode
129129
.parse()
130130
.map_err(|_|
131-
exec_datafusion_err!("Failed to parse input: {mode}. Valid options are disabled, enabled")
131+
exec_datafusion_err!("Failed to parse input: {mode}. Valid options are disabled, summary, trace")
132132
)?;
133133
print_options
134134
.instrumented_registry
@@ -165,7 +165,7 @@ impl Command {
165165
("\\pset [NAME [VALUE]]", "set table output option\n(format)")
166166
}
167167
Self::ObjectStoreProfileMode(_) => (
168-
"\\object_store_profiling (disabled|enabled)",
168+
"\\object_store_profiling (disabled|summary|trace)",
169169
"print or set object store profile mode",
170170
),
171171
}
@@ -312,13 +312,22 @@ mod tests {
312312
InstrumentedObjectStoreMode::default()
313313
);
314314

315-
cmd = "object_store_profiling enabled"
315+
cmd = "object_store_profiling summary"
316316
.parse()
317317
.expect("expected parse to succeed");
318318
assert!(cmd.execute(&ctx, &mut print_options).await.is_ok());
319319
assert_eq!(
320320
print_options.instrumented_registry.instrument_mode(),
321-
InstrumentedObjectStoreMode::Enabled
321+
InstrumentedObjectStoreMode::Summary
322+
);
323+
324+
cmd = "object_store_profiling trace"
325+
.parse()
326+
.expect("expected parse to succeed");
327+
assert!(cmd.execute(&ctx, &mut print_options).await.is_ok());
328+
assert_eq!(
329+
print_options.instrumented_registry.instrument_mode(),
330+
InstrumentedObjectStoreMode::Trace
322331
);
323332

324333
cmd = "object_store_profiling does_not_exist"

datafusion-cli/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ struct Args {
151151

152152
#[clap(
153153
long,
154-
help = "Specify the default object_store_profiling mode, defaults to 'disabled'.\n[possible values: disabled, enabled]",
154+
help = "Specify the default object_store_profiling mode, defaults to 'disabled'.\n[possible values: disabled, summary, trace]",
155155
default_value_t = InstrumentedObjectStoreMode::Disabled
156156
)]
157157
object_store_profiling: InstrumentedObjectStoreMode,

datafusion-cli/src/object_storage/instrumented.rs

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,10 @@ pub enum InstrumentedObjectStoreMode {
4848
/// Disable collection of profiling data
4949
#[default]
5050
Disabled,
51-
/// Enable collection of profiling data
52-
Enabled,
51+
/// Enable collection of profiling data and output a summary
52+
Summary,
53+
/// Enable collection of profiling data and output a summary and all details
54+
Trace,
5355
}
5456

5557
impl fmt::Display for InstrumentedObjectStoreMode {
@@ -64,7 +66,8 @@ impl FromStr for InstrumentedObjectStoreMode {
6466
fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
6567
match s.to_lowercase().as_str() {
6668
"disabled" => Ok(Self::Disabled),
67-
"enabled" => Ok(Self::Enabled),
69+
"summary" => Ok(Self::Summary),
70+
"trace" => Ok(Self::Trace),
6871
_ => Err(DataFusionError::Execution(format!("Unrecognized mode {s}"))),
6972
}
7073
}
@@ -73,7 +76,8 @@ impl FromStr for InstrumentedObjectStoreMode {
7376
impl From<u8> for InstrumentedObjectStoreMode {
7477
fn from(value: u8) -> Self {
7578
match value {
76-
1 => InstrumentedObjectStoreMode::Enabled,
79+
1 => InstrumentedObjectStoreMode::Summary,
80+
2 => InstrumentedObjectStoreMode::Trace,
7781
_ => InstrumentedObjectStoreMode::Disabled,
7882
}
7983
}
@@ -434,16 +438,21 @@ mod tests {
434438
InstrumentedObjectStoreMode::Disabled
435439
));
436440
assert!(matches!(
437-
"EnABlEd".parse().unwrap(),
438-
InstrumentedObjectStoreMode::Enabled
441+
"SUmMaRy".parse().unwrap(),
442+
InstrumentedObjectStoreMode::Summary
443+
));
444+
assert!(matches!(
445+
"TRaCe".parse().unwrap(),
446+
InstrumentedObjectStoreMode::Trace
439447
));
440448
assert!("does_not_exist"
441449
.parse::<InstrumentedObjectStoreMode>()
442450
.is_err());
443451

444452
assert!(matches!(0.into(), InstrumentedObjectStoreMode::Disabled));
445-
assert!(matches!(1.into(), InstrumentedObjectStoreMode::Enabled));
446-
assert!(matches!(2.into(), InstrumentedObjectStoreMode::Disabled));
453+
assert!(matches!(1.into(), InstrumentedObjectStoreMode::Summary));
454+
assert!(matches!(2.into(), InstrumentedObjectStoreMode::Trace));
455+
assert!(matches!(3.into(), InstrumentedObjectStoreMode::Disabled));
447456
}
448457

449458
#[test]
@@ -455,8 +464,8 @@ mod tests {
455464
InstrumentedObjectStoreMode::default()
456465
);
457466

458-
reg = reg.with_profile_mode(InstrumentedObjectStoreMode::Enabled);
459-
assert_eq!(reg.instrument_mode(), InstrumentedObjectStoreMode::Enabled);
467+
reg = reg.with_profile_mode(InstrumentedObjectStoreMode::Trace);
468+
assert_eq!(reg.instrument_mode(), InstrumentedObjectStoreMode::Trace);
460469

461470
let store = object_store::memory::InMemory::new();
462471
let url = "mem://test".parse().unwrap();
@@ -484,7 +493,7 @@ mod tests {
484493
let _ = instrumented.get(&path).await.unwrap();
485494
assert!(instrumented.requests.lock().is_empty());
486495

487-
instrumented.set_instrument_mode(InstrumentedObjectStoreMode::Enabled);
496+
instrumented.set_instrument_mode(InstrumentedObjectStoreMode::Trace);
488497
assert!(instrumented.requests.lock().is_empty());
489498
let _ = instrumented.get(&path).await.unwrap();
490499
assert_eq!(instrumented.requests.lock().len(), 1);

datafusion-cli/src/print_options.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -188,20 +188,21 @@ impl PrintOptions {
188188
if !self.quiet {
189189
writeln!(writer, "{formatted_exec_details}")?;
190190

191-
if self.instrumented_registry.instrument_mode()
192-
!= InstrumentedObjectStoreMode::Disabled
193-
{
191+
let instrument_mode = self.instrumented_registry.instrument_mode();
192+
if instrument_mode != InstrumentedObjectStoreMode::Disabled {
194193
writeln!(writer, "{OBJECT_STORE_PROFILING_HEADER}")?;
195194
for store in self.instrumented_registry.stores() {
196195
let requests = store.take_requests();
197196

198197
if !requests.is_empty() {
199198
writeln!(writer, "{store}")?;
200-
for req in requests.iter() {
201-
writeln!(writer, "{req}")?;
199+
if instrument_mode == InstrumentedObjectStoreMode::Trace {
200+
for req in requests.iter() {
201+
writeln!(writer, "{req}")?;
202+
}
203+
// Add an extra blank line to help visually organize the output
204+
writeln!(writer)?;
202205
}
203-
// Add an extra blank line to help visually organize the output
204-
writeln!(writer)?;
205206

206207
writeln!(writer, "Summaries:")?;
207208
let summaries = RequestSummary::summarize_by_operation(&requests);
@@ -252,7 +253,7 @@ mod tests {
252253
print_output.clear();
253254
print_options
254255
.instrumented_registry
255-
.set_instrument_mode(InstrumentedObjectStoreMode::Enabled);
256+
.set_instrument_mode(InstrumentedObjectStoreMode::Trace);
256257
print_options.write_output(&mut print_output, exec_out.clone())?;
257258
let out_str: String = print_output
258259
.clone()

datafusion-cli/tests/cli_integration.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,8 +434,11 @@ LOCATION 's3://data/cars.csv';
434434
435435
-- Initial query should not show any profiling as the object store is not instrumented yet
436436
SELECT * from CARS LIMIT 1;
437-
\object_store_profiling enabled
438-
-- Query again to see the profiling output
437+
\object_store_profiling trace
438+
-- Query again to see the full profiling output
439+
SELECT * from CARS LIMIT 1;
440+
\object_store_profiling summary
441+
-- Query again to see the summarized profiling output
439442
SELECT * from CARS LIMIT 1;
440443
\object_store_profiling disabled
441444
-- Final query should not show any profiling as we disabled it again

datafusion-cli/tests/snapshots/object_store_profiling@s3_url_fallback.snap

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ info:
88
AWS_ALLOW_HTTP: "true"
99
AWS_ENDPOINT: "http://localhost:55031"
1010
AWS_SECRET_ACCESS_KEY: TEST-DataFusionPassword
11-
stdin: "\n CREATE EXTERNAL TABLE CARS\nSTORED AS CSV\nLOCATION 's3://data/cars.csv';\n\n-- Initial query should not show any profiling as the object store is not instrumented yet\nSELECT * from CARS LIMIT 1;\n\\object_store_profiling enabled\n-- Query again to see the profiling output\nSELECT * from CARS LIMIT 1;\n\\object_store_profiling disabled\n-- Final query should not show any profiling as we disabled it again\nSELECT * from CARS LIMIT 1;\n"
11+
stdin: "\n CREATE EXTERNAL TABLE CARS\nSTORED AS CSV\nLOCATION 's3://data/cars.csv';\n\n-- Initial query should not show any profiling as the object store is not instrumented yet\nSELECT * from CARS LIMIT 1;\n\\object_store_profiling trace\n-- Query again to see the full profiling output\nSELECT * from CARS LIMIT 1;\n\\object_store_profiling summary\n-- Query again to see the summarized profiling output\nSELECT * from CARS LIMIT 1;\n\\object_store_profiling disabled\n-- Final query should not show any profiling as we disabled it again\nSELECT * from CARS LIMIT 1;\n"
1212
snapshot_kind: text
1313
---
1414
success: true
@@ -26,7 +26,7 @@ exit_code: 0
2626
1 row(s) fetched.
2727
[ELAPSED]
2828

29-
ObjectStore Profile mode set to Enabled
29+
ObjectStore Profile mode set to Trace
3030
+-----+-------+---------------------+
3131
| car | speed | time |
3232
+-----+-------+---------------------+
@@ -36,7 +36,7 @@ ObjectStore Profile mode set to Enabled
3636
[ELAPSED]
3737

3838
Object Store Profiling
39-
Instrumented Object Store: instrument_mode: Enabled, inner: AmazonS3(data)
39+
Instrumented Object Store: instrument_mode: Trace, inner: AmazonS3(data)
4040
<TIMESTAMP> operation=Get duration=[DURATION] size=1006 path=cars.csv
4141

4242
Summaries:
@@ -50,6 +50,28 @@ size max: 1006 B
5050
size avg: 1006 B
5151
size sum: 1006 B
5252

53+
ObjectStore Profile mode set to Summary
54+
+-----+-------+---------------------+
55+
| car | speed | time |
56+
+-----+-------+---------------------+
57+
| red | 20.0 | 1996-04-12T12:05:03 |
58+
+-----+-------+---------------------+
59+
1 row(s) fetched.
60+
[ELAPSED]
61+
62+
Object Store Profiling
63+
Instrumented Object Store: instrument_mode: Summary, inner: AmazonS3(data)
64+
Summaries:
65+
Get
66+
count: 1
67+
[SUMMARY_DURATION]
68+
[SUMMARY_DURATION]
69+
[SUMMARY_DURATION]
70+
size min: 1006 B
71+
size max: 1006 B
72+
size avg: 1006 B
73+
size sum: 1006 B
74+
5375
ObjectStore Profile mode set to Disabled
5476
+-----+-------+---------------------+
5577
| car | speed | time |

docs/source/user-guide/cli/usage.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ OPTIONS:
6565
6666
--object-store-profiling <OBJECT_STORE_PROFILING>
6767
Specify the default object_store_profiling mode, defaults to 'disabled'.
68-
[possible values: disabled, enabled] [default: Disabled]
68+
[possible values: disabled, summary, trace] [default: Disabled]
6969
7070
-p, --data-path <DATA_PATH>
7171
Path to your data, default to current directory
@@ -129,7 +129,7 @@ Available commands inside DataFusion CLI are:
129129
- Object Store Profiling Mode
130130
131131
```bash
132-
> \object_store_profiling [disabled|enabled]
132+
> \object_store_profiling [disabled|summary|trace]
133133
```
134134
135135
## Supported SQL

0 commit comments

Comments
 (0)