#3651: Don't discard return values in MakeCallWithTiming when the meter doesn't return a histogram

src/aws-cpp-sdk-core/include/smithy/tracing/TracingUtils.h

@@ -19,6 +19,16 @@ namespace smithy {
              * A utility class for common tracing activities.
              */
             class SMITHY_API TracingUtils {
+                using SteadyTime = std::chrono::steady_clock::time_point;
+
+                static void RecordExecutionDuration(
+                    SteadyTime before,
+                    SteadyTime after,
+                    Aws::String metricName,
+                    const Meter &meter,
+                    Aws::Map<Aws::String, Aws::String> attributes,
+                    Aws::String description
+                );
             public:
                 TracingUtils() = default;
 
@@ -65,13 +75,7 @@ namespace smithy {
                     auto before = std::chrono::steady_clock::now();
                     auto returnValue = func();
                     auto after = std::chrono::steady_clock::now();
-                    auto duration = std::chrono::duration_cast<std::chrono::microseconds>(after - before).count();
-                    auto histogram = meter.CreateHistogram(metricName, MICROSECOND_METRIC_TYPE, description);
-                    if (!histogram) {
-                        AWS_LOG_ERROR("TracingUtil", "Failed to create histogram");
-                        return {};
-                    }
-                    histogram->record((double) duration, std::forward<Aws::Map<Aws::String, Aws::String>>(attributes));
+                    RecordExecutionDuration(before, after, std::move(metricName), meter, std::move(attributes), std::move(description));
                     return returnValue;
                 }
 
@@ -93,13 +97,7 @@ namespace smithy {
                     auto before = std::chrono::steady_clock::now();
                     func();
                     auto after = std::chrono::steady_clock::now();
-                    auto duration = std::chrono::duration_cast<std::chrono::microseconds>(after - before).count();
-                    auto histogram = meter.CreateHistogram(std::move(metricName), MICROSECOND_METRIC_TYPE, std::move(description));
-                    if (!histogram) {
-                        AWS_LOG_ERROR("TracingUtil", "Failed to create histogram");
-                        return;
-                    }
-                    histogram->record((double) duration, std::forward<Aws::Map<Aws::String, Aws::String>>(attributes));
+                    RecordExecutionDuration(before, after, std::move(metricName), meter, std::move(attributes), std::move(description));
                 }
 
                 /**
@@ -122,8 +120,9 @@ namespace smithy {
                                 std::move(description));
                             if (!histogram) {
                                 AWS_LOG_ERROR("TracingUtil", "Failed to create histogram");
+                            } else {
+                                histogram->record((double) entry.second, attributes);
                             }
-                            histogram->record((double) entry.second, attributes);
                         }
                     }
                 }

src/aws-cpp-sdk-core/source/smithy/tracing/TracingUtils.cpp

@@ -28,4 +28,21 @@ const char TracingUtils::SMITHY_METRICS_CONNECT_DURATION[] = "smithy.client.http
 const char TracingUtils::SMITHY_METRICS_SSL_DURATION[] = "smithy.client.http.ssl_duration";
 const char TracingUtils::SMITHY_METRICS_DOWNLOAD_SPEED_METRIC[] = "smithy.client.http.download_speed";
 const char TracingUtils::SMITHY_METRICS_UPLOAD_SPEED_METRIC[] = "smithy.client.http.upload_speed";
-const char TracingUtils::SMITHY_METRICS_UNKNOWN_METRIC[] = "smithy.client.http.unknown_metric";
+const char TracingUtils::SMITHY_METRICS_UNKNOWN_METRIC[] = "smithy.client.http.unknown_metric";
+
+void TracingUtils::RecordExecutionDuration(
+    SteadyTime before,
+    SteadyTime after,
+    Aws::String metricName,
+    const Meter &meter,
+    Aws::Map<Aws::String, Aws::String> attributes,
+    Aws::String description
+) {
+    auto duration = std::chrono::duration_cast<std::chrono::microseconds>(after - before).count();
+    auto histogram = meter.CreateHistogram(std::move(metricName), MICROSECOND_METRIC_TYPE, description);
+    if (!histogram) {
+        AWS_LOG_ERROR("TracingUtil", "Failed to create histogram");
+    } else {
+        histogram->record((double) duration, std::move(attributes));
+    }
+}

tests/aws-cpp-sdk-core-tests/smithy/tracing/SmithyTracingUtilsTest.cpp

@@ -0,0 +1,55 @@
+/**
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0.
+ */
+#include <aws/testing/AwsCppSdkGTestSuite.h>
+#include <smithy/tracing/NoopTracerProvider.h>
+#include <smithy/tracing/NoopMeterProvider.h>
+#include "smithy/tracing/TracingUtils.h"
+
+using namespace smithy::components::tracing;
+
+static const char *ALLOC_TAG = "SMITHY_TRACING_ALLOC";
+
+class SmithyTracingUtilsTest : public Aws::Testing::AwsCppSdkGTestSuite {
+};
+
+TEST_F(SmithyTracingUtilsTest, TestMakeCallWithTiming) {
+    auto provider = Aws::MakeUnique<NoopMeterProvider>(ALLOC_TAG);
+    auto meter = provider->GetMeter("scope", {});
+
+    auto value = TracingUtils::MakeCallWithTiming<int>(
+        []() { return 42; },
+        TracingUtils::SMITHY_CLIENT_DURATION_METRIC,
+        *meter,
+        { { TracingUtils::SMITHY_SYSTEM_DIMENSION, TracingUtils::SMITHY_METHOD_AWS_VALUE } },
+        "test duration metric"
+    );
+
+    ASSERT_EQ(value, 42);
+}
+
+class NullMeter : public NoopMeter {
+public:
+    Aws::UniquePtr<Histogram> CreateHistogram(Aws::String name, Aws::String units, Aws::String description) const override {
+        AWS_UNREFERENCED_PARAM(name);
+        AWS_UNREFERENCED_PARAM(units);
+        AWS_UNREFERENCED_PARAM(description);
+
+        return nullptr;
+    }
+};
+
+TEST_F(SmithyTracingUtilsTest, TestMakeCallWithTimingNullHistogram) {
+    auto meter = Aws::MakeUnique<NullMeter>(ALLOC_TAG);
+
+    auto value = TracingUtils::MakeCallWithTiming<int>(
+        []() { return 42; },
+        TracingUtils::SMITHY_CLIENT_DURATION_METRIC,
+        *meter,
+        { { TracingUtils::SMITHY_SYSTEM_DIMENSION, TracingUtils::SMITHY_METHOD_AWS_VALUE } },
+        "test duration metric"
+    );
+
+    ASSERT_EQ(value, 42);
+}