address review comments

Author: Ariel Ben-Yehuda

CHANGELOG.md

@@ -10,7 +10,6 @@ All notable changes to this project will be documented in this file.
 
 - add support for stopping the profiler
 - make native memory profiling interval configurable
-- add Cargo features to allow not enabling AWS/reqwest default features
 
 ### Other
 

examples/simple/main.rs

@@ -29,23 +29,28 @@ pub fn set_up_tracing() {
         .init();
 }
 
+
+#[derive(Clone, Debug, Parser)]
+struct S3BucketArgs {
+    #[arg(long, requires = "bucket_owner", requires = "profiling_group")]
+    bucket: Option<String>,
+    #[arg(long)]
+    bucket_owner: Option<String>,
+    #[arg(long)]
+    profiling_group: Option<String>,
+}
+
 /// Simple program to test the profiler agent
-#[derive(Parser, Debug)]
+#[derive(Debug, Parser)]
 #[command(group(
     ArgGroup::new("options")
         .required(true)
         .args(["local", "bucket"]),
 ))]
 struct Args {
     #[cfg(feature = "s3-no-defaults")]
-    #[arg(long)]
-    profiling_group: Option<String>,
-    #[cfg(feature = "s3-no-defaults")]
-    #[arg(long)]
-    bucket_owner: Option<String>,
-    #[cfg(feature = "s3-no-defaults")]
-    #[arg(long, requires = "bucket_owner", requires = "profiling_group")]
-    bucket: Option<String>,
+    #[command(flatten)]
+    bucket_args: S3BucketArgs,
     #[arg(long)]
     local: Option<String>,
     #[arg(long)]
@@ -60,6 +65,18 @@ struct Args {
     native_mem: Option<String>,
 }
 
+impl Args {
+    #[cfg(feature = "s3-no-defaults")]
+    fn s3_bucket_args(&self) -> S3BucketArgs {
+        self.bucket_args.clone()
+    }
+
+    #[cfg(not(feature = "s3-no-defaults"))]
+    fn s3_bucket_args(&self) -> S3BucketArgs {
+        BucketArgs::default()
+    }
+}
+
 #[allow(unexpected_cfgs)]
 pub fn main() -> anyhow::Result<()> {
     let args = Args::parse();
@@ -85,25 +102,16 @@ async fn main_internal(args: Args) -> Result<(), anyhow::Error> {
 
     let profiler = ProfilerBuilder::default();
 
-    #[cfg(feature = "s3-no-defaults")]
-    let bucket_name = args.bucket;
-    #[cfg(not(feature = "s3-no-defaults"))]
-    let bucket_name: Option<String> = None;
-    #[cfg(feature = "s3-no-defaults")]
-    let bucket_owner = args.bucket_owner;
-    #[cfg(not(feature = "s3-no-defaults"))]
-    let bucket_owner: Option<String> = None;
-    #[cfg(feature = "s3-no-defaults")]
-    let profiling_group_name = args.profiling_group;
-    #[cfg(not(feature = "s3-no-defaults"))]
-    let profiling_group_name: Option<String> = None;
-
-    let profiler = match (args.local, bucket_name, bucket_owner, profiling_group_name) {
-        (Some(local), _, _, _) => profiler
+    let profiler = match (&args.local, args.s3_bucket_args()) {
+        (Some(local), S3BucketArgs { .. }) => profiler
             .with_reporter(LocalReporter::new(local))
             .with_custom_agent_metadata(AgentMetadata::Other),
         #[cfg(feature = "s3-no-defaults")]
-        (_, Some(bucket_name), Some(bucket_owner), Some(profiling_group_name)) => profiler
+        (_, S3BucketArgs {
+            bucket: Some(bucket_name),
+            bucket_owner: Some(bucket_owner),
+            profiling_group: Some(profiling_group_name)
+        }) => profiler
             .with_reporter(S3Reporter::new(S3ReporterConfig {
                 sdk_config: &aws_config::defaults(BehaviorVersion::latest()).load().await,
                 bucket_owner,