diff --git a/.changelog/bugfix-checksum-user-agent-features.json b/.changelog/bugfix-checksum-user-agent-features.json new file mode 100644 index 000000000000..2bb74f481dda --- /dev/null +++ b/.changelog/bugfix-checksum-user-agent-features.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "aws-cpp-sdk-core", + "contributor": "jonathan343", + "description": "Fix the flexible checksum business metrics reported in the User-Agent. CRC32 was reported as CRC32C, CRC64NVME was never reported, and the SHA512, XXHASH3, XXHASH64 and XXHASH128 metric IDs were wrong." +} diff --git a/src/aws-cpp-sdk-core/include/smithy/client/features/ChecksumInterceptor.h b/src/aws-cpp-sdk-core/include/smithy/client/features/ChecksumInterceptor.h index 85a0e835a44f..4f361e0f4ca9 100644 --- a/src/aws-cpp-sdk-core/include/smithy/client/features/ChecksumInterceptor.h +++ b/src/aws-cpp-sdk-core/include/smithy/client/features/ChecksumInterceptor.h @@ -151,10 +151,10 @@ class ChecksumInterceptor : public smithy::interceptor::Interceptor { private: void addChecksumFeatureForChecksumName(const Aws::String& checksumName, const Aws::AmazonWebServiceRequest& request) { if (checksumName == "crc32") { - request.AddUserAgentFeature(Aws::Client::UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_CRC32C); + request.AddUserAgentFeature(Aws::Client::UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_CRC32); } else if (checksumName == "crc32c") { request.AddUserAgentFeature(Aws::Client::UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_CRC32C); - } else if (checksumName == "crc64") { + } else if (checksumName == "crc64nvme") { request.AddUserAgentFeature(Aws::Client::UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_CRC64); } else if (checksumName == "sha1") { request.AddUserAgentFeature(Aws::Client::UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_SHA1); diff --git a/src/aws-cpp-sdk-core/source/client/UserAgent.cpp b/src/aws-cpp-sdk-core/source/client/UserAgent.cpp index 098688119cb2..07559bfaee59 100644 --- a/src/aws-cpp-sdk-core/source/client/UserAgent.cpp +++ b/src/aws-cpp-sdk-core/source/client/UserAgent.cpp @@ -55,10 +55,10 @@ const std::pair BUSINESS_METRIC_MAPPING[] = { {UserAgentFeature::CREDENTIALS_LOGIN, "AD"}, {UserAgentFeature::PROTOCOL_RPC_V2_CBOR, "M"}, {UserAgentFeature::BEARER_SERVICE_ENV_VARS, "3"}, - {UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_SHA512, "AE"}, - {UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_XXHASH64, "AG"}, - {UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_XXHASH3, "AF"}, - {UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_XXHASH128, "AH"}, + {UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_SHA512, "AF"}, + {UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_XXHASH64, "AH"}, + {UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_XXHASH3, "AG"}, + {UserAgentFeature::FLEXIBLE_CHECKSUMS_REQ_XXHASH128, "AI"}, {UserAgentFeature::PAGINATOR, "C"}, {UserAgentFeature::WAITER, "B"}, }; diff --git a/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp b/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp index 69c3ab632c67..da803b73c8c5 100644 --- a/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp +++ b/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp @@ -20,8 +20,10 @@ #include #include #include +#include #include +#include #include using namespace Aws; @@ -441,6 +443,71 @@ TEST_F(S3UnitTest, PutObjectShouldHaveCorrectUserAgent) { EXPECT_TRUE(archMetadata < businessMetrics); } +namespace { +struct ChecksumFeatureTestCase { + ChecksumAlgorithm algorithm; + std::function setChecksum; + Aws::String metric; +}; +} // namespace + +TEST_F(S3UnitTest, PutObjectShouldTrackChecksumAlgorithmFeature) { + // FLEXIBLE_CHECKSUMS_REQ_* metric values, from the AWS SDK business metrics registry. + const Aws::Vector testCases{ + {ChecksumAlgorithm::CRC32, [](PutObjectRequest& r) { r.SetChecksumCRC32("Y2hlY2tzdW0="); }, "U"}, + {ChecksumAlgorithm::CRC32C, [](PutObjectRequest& r) { r.SetChecksumCRC32C("Y2hlY2tzdW0="); }, "V"}, + {ChecksumAlgorithm::CRC64NVME, [](PutObjectRequest& r) { r.SetChecksumCRC64NVME("Y2hlY2tzdW0="); }, "W"}, + {ChecksumAlgorithm::SHA1, [](PutObjectRequest& r) { r.SetChecksumSHA1("Y2hlY2tzdW0="); }, "X"}, + {ChecksumAlgorithm::SHA256, [](PutObjectRequest& r) { r.SetChecksumSHA256("Y2hlY2tzdW0="); }, "Y"}, + {ChecksumAlgorithm::SHA512, [](PutObjectRequest& r) { r.SetChecksumSHA512("Y2hlY2tzdW0="); }, "AF"}, + {ChecksumAlgorithm::XXHASH3, [](PutObjectRequest& r) { r.SetChecksumXXHASH3("Y2hlY2tzdW0="); }, "AG"}, + {ChecksumAlgorithm::XXHASH64, [](PutObjectRequest& r) { r.SetChecksumXXHASH64("Y2hlY2tzdW0="); }, "AH"}, + {ChecksumAlgorithm::XXHASH128, [](PutObjectRequest& r) { r.SetChecksumXXHASH128("Y2hlY2tzdW0="); }, "AI"}, + }; + Aws::Vector allMetrics; + for (const auto& testCase : testCases) { + allMetrics.push_back(testCase.metric); + } + + for (const auto& testCase : testCases) { + // The SDK either calculates the checksum, or sends the one set on the request. + for (const bool provided : {false, true}) { + SCOPED_TRACE(Aws::String(ChecksumAlgorithmMapper::GetNameForChecksumAlgorithm(testCase.algorithm)) + + (provided ? " provided" : " calculated")); + // The mock client is shared by the suite, so start each case clean. + _mockHttpClient->Reset(); + auto request = PutObjectRequest() + .WithBucket("big-boss") + .WithKey("snake-eater") + .WithChecksumAlgorithm(testCase.algorithm); + if (provided) { + testCase.setChecksum(request); + } + std::shared_ptr body = Aws::MakeShared(ALLOCATION_TAG, "phantom pain"); + request.SetBody(body); + + auto mockRequest = Aws::MakeShared(ALLOCATION_TAG, "outer.heaven/mother-base", HttpMethod::HTTP_PUT); + mockRequest->SetResponseStreamFactory([]() -> IOStream* { + return Aws::New(ALLOCATION_TAG, "", std::ios_base::in | std::ios_base::binary); + }); + auto mockResponse = Aws::MakeShared(ALLOCATION_TAG, mockRequest); + mockResponse->SetResponseCode(HttpResponseCode::OK); + _mockHttpClient->AddResponseToReturn(mockResponse); + + const auto response = _s3Client->PutObject(request); + EXPECT_TRUE(response.IsSuccess()); + ASSERT_EQ(1u, _mockHttpClient->GetAllRequestsMade().size()); + + // Exactly one checksum algorithm metric is reported, and it's the right one. + const auto metrics = GetFeaturesForRequest(_mockHttpClient->GetMostRecentHttpRequest()); + for (const auto& metric : allMetrics) { + const bool reported = std::find(metrics.begin(), metrics.end(), metric) != metrics.end(); + EXPECT_EQ(metric == testCase.metric, reported) << "metric " << metric; + } + } + } +} + TEST_F(S3UnitTest, PutObjectShouldOverrideUserAgent) { auto request = PutObjectRequest() .WithBucket("ocelot")