Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable tracing S3 id pairs and fix SNS naming issue. #168

Merged
merged 4 commits into from
Dec 29, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sdk/src/Core/AWSXRayRecorder.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="AWSSDK.Core" Version="[3.3.25.1,4.0)" />
<PackageReference Include="AWSSDK.Core" Version="3.3.25.1" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
Expand Down
17 changes: 17 additions & 0 deletions sdk/src/Handlers/AwsSdk/Internal/AWSXRaySDKUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ internal class AWSXRaySDKUtils
{
private static readonly String XRayServiceName = "XRay";
private static readonly ISet<String> WhitelistedOperations = new HashSet<String> { "GetSamplingRules", "GetSamplingTargets" };

// Collection to uniform service names across X-Ray SDKs.
private static readonly Dictionary<string, string> FormattedServiceNames = new Dictionary<string, string>()
{
{ "SimpleNotificationService" , "SNS" },
};

internal static bool IsBlacklistedOperation(String serviceName, string operation)
{
if (string.Equals(serviceName, XRayServiceName) && WhitelistedOperations.Contains(operation))
Expand All @@ -34,5 +41,15 @@ internal static bool IsBlacklistedOperation(String serviceName, string operation
}
return false;
}

internal static string FormatServiceName(string serviceName)
{
if (FormattedServiceNames.TryGetValue(serviceName, out string formattedName))
{
return formattedName;
}

return serviceName;
}
}
}
26 changes: 19 additions & 7 deletions sdk/src/Handlers/AwsSdk/Internal/XRayPipelineHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ public class XRayPipelineHandler : PipelineHandler
private const string DefaultAwsWhitelistManifestResourceName = "Amazon.XRay.Recorder.Handlers.AwsSdk.DefaultAWSWhitelist.json";
private static readonly Logger _logger = Runtime.Internal.Util.Logger.GetLogger(typeof(AWSXRayRecorder));
private AWSXRayRecorder _recorder;
private const String S3RequestIdHeaderKey = "x-amz-request-id";
private const String S3ExtendedRequestIdHeaderKey = "x-amz-id-2";
private const String ExtendedRquestIdSegmentKey = "id_2";

/// <summary>
/// Gets AWS service manifest of operation parameter whitelist.
Expand Down Expand Up @@ -319,7 +316,8 @@ private void ProcessBeginRequest(IExecutionContext executionContext)
_recorder.TraceContext.HandleEntityMissing(_recorder, e, "Cannot get entity while processing AWS SDK request");
}

_recorder.BeginSubsegment(RemoveAmazonPrefixFromServiceName(request.ServiceName));
var serviceName = RemoveAmazonPrefixFromServiceName(request.ServiceName);
_recorder.BeginSubsegment(AWSXRaySDKUtils.FormatServiceName(serviceName));
_recorder.SetNamespace("aws");

entity = entity == null ? null : _recorder.GetEntity();
Expand Down Expand Up @@ -380,20 +378,27 @@ private void ProcessEndRequest(IExecutionContext executionContext)
// s3 doesn't follow request header id convention
else
{
if (requestContext.Request.Headers.TryGetValue(S3RequestIdHeaderKey, out requestId))
if (requestContext.Request.Headers.TryGetValue("x-amz-request-id", out requestId))
{
subsegment.Aws["request_id"] = requestId;
}

if (requestContext.Request.Headers.TryGetValue(S3ExtendedRequestIdHeaderKey, out requestId))
if (requestContext.Request.Headers.TryGetValue("x-amz-id-2", out requestId))
{
subsegment.Aws[ExtendedRquestIdSegmentKey] = requestId;
subsegment.Aws["id_2"] = requestId;
}
}
}
else
{
subsegment.Aws["request_id"] = responseContext.Response.ResponseMetadata.RequestId;

// try getting x-amz-id-2 if dealing with s3 request
if (responseContext.Response.ResponseMetadata.Metadata.TryGetValue("x-amz-id-2", out string extendedRequestId))
{
subsegment.Aws["id_2"] = extendedRequestId;
}

AddResponseSpecificInformation(serviceName, operation, responseContext.Response, subsegment.Aws);
}

Expand Down Expand Up @@ -451,6 +456,13 @@ private void ProcessException(AmazonServiceException ex, Entity subsegment)
_recorder.AddHttpInformation("response", responseAttributes);

subsegment.Aws["request_id"] = ex.RequestId;

// AmazonId2 property in AmazonS3Exception corresponds to the x-amz-id-2 Http header
var property = ex.GetType().GetProperty("AmazonId2");
if (property != null)
{
subsegment.Aws["id_2"] = (string)property.GetValue(ex, null);
}
}

private void AddRequestSpecificInformation(string serviceName, string operation, AmazonWebServiceRequest request, IDictionary<string, object> aws)
Expand Down
45 changes: 37 additions & 8 deletions sdk/test/UnitTests/AWSSDKHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
using Amazon.Lambda.Model;
using Amazon.Runtime;
using Amazon.S3;
using Amazon.SimpleNotificationService;
using Amazon.XRay.Recorder.Core;
using Amazon.XRay.Recorder.Core.Internal.Utils;
using Amazon.XRay.Recorder.Handlers.AwsSdk;
Expand Down Expand Up @@ -80,7 +81,7 @@ public void TestContextMissingStrategyForAWSSDKHandler()
AWSXRayRecorder.Instance.ContextMissingStrategy = Core.Strategies.ContextMissingStrategy.LOG_ERROR;
AWSSDKHandler.RegisterXRayForAllServices();
var dynamo = new AmazonDynamoDBClient(new AnonymousAWSCredentials(), RegionEndpoint.USEast1);
CustomResponses.SetResponse(dynamo, null, null, true);
CustomResponses.SetResponse(dynamo, null, null, null, true);

AWSXRayRecorder.Instance.BeginSegment("test dynamo", TraceId);
var segment = AWSXRayRecorder.Instance.TraceContext.GetEntity();
Expand All @@ -101,7 +102,7 @@ public void TestS3SubsegmentNameIsCorrectForAWSSDKHandler()
{
AWSSDKHandler.RegisterXRay<IAmazonS3>();
var s3 = new AmazonS3Client(new AnonymousAWSCredentials(), RegionEndpoint.USEast1);
CustomResponses.SetResponse(s3, null, null, true);
CustomResponses.SetResponse(s3, null, null, "TestAmazonId", true);

_recorder.BeginSegment("test s3", TraceId);
#if NET45
Expand All @@ -115,6 +116,7 @@ public void TestS3SubsegmentNameIsCorrectForAWSSDKHandler()
Assert.IsTrue(segment.Subsegments[0].Aws.ContainsKey("version_id"));
Assert.AreEqual(segment.Subsegments[0].Aws["bucket_name"], "testBucket");
Assert.AreEqual(segment.Subsegments[0].Aws["operation"], "GetObject");
Assert.AreEqual(segment.Subsegments[0].Aws["id_2"], "TestAmazonId");
}

[TestMethod]
Expand All @@ -126,7 +128,7 @@ public void TestDynamoDbClient()
using (var client = new AmazonDynamoDBClient(new AnonymousAWSCredentials(), RegionEndpoint.USEast1))
{
string requestId = @"fakerequ-esti-dfak-ereq-uestidfakere";
CustomResponses.SetResponse(client, null, requestId, true);
CustomResponses.SetResponse(client, null, requestId, null, true);

_recorder.BeginSegment("test", TraceId);
#if NET45
Expand Down Expand Up @@ -184,7 +186,7 @@ public void TestRequestResponseParameterAndDescriptorForAWSSDKHandler()
{
using (var client = new AmazonDynamoDBClient(new AnonymousAWSCredentials(), RegionEndpoint.USEast1))
{
CustomResponses.SetResponse(client, null, null, true);
CustomResponses.SetResponse(client, null, null, null, true);
_recorder.BeginSegment("test", TraceId);
var segment = AWSXRayRecorder.Instance.TraceContext.GetEntity();

Expand Down Expand Up @@ -255,7 +257,7 @@ await client.GetItemAsync(
public void TestDynamoSubsegmentNameIsCorrectForAWSSDKHandler()
{
var dynamo = new AmazonDynamoDBClient(new AnonymousAWSCredentials(), RegionEndpoint.USEast1);
CustomResponses.SetResponse(dynamo, null, null, true);
CustomResponses.SetResponse(dynamo, null, null, null, true);
_recorder.BeginSegment("test dynamo", TraceId);
#if NET45
dynamo.ListTables();
Expand All @@ -272,7 +274,7 @@ public void TestDynamoSubsegmentNameIsCorrectForAWSSDKHandler()
public void TestManifestFileNoLambda() //At this point, current manifest file doen't contain Lambda service.
{
var lambda = new AmazonLambdaClient(new AnonymousAWSCredentials(), RegionEndpoint.USEast1);
CustomResponses.SetResponse(lambda, null, null, true);
CustomResponses.SetResponse(lambda, null, null, null, true);
_recorder.BeginSegment("lambda", TraceId);
#if NET45
lambda.Invoke(new InvokeRequest
Expand All @@ -296,7 +298,7 @@ public void TestLambdaInvokeSubsegmentContainsFunctionNameForAWSSDKHandler()
String temp_path = $"JSONs{Path.DirectorySeparatorChar}AWSRequestInfoWithLambda.json"; //registering manifest file with Lambda
AWSSDKHandler.RegisterXRayManifest(temp_path);
var lambda = new AmazonLambdaClient(new AnonymousAWSCredentials(), RegionEndpoint.USEast1);
CustomResponses.SetResponse(lambda, null, null, true);
CustomResponses.SetResponse(lambda, null, null, null, true);
_recorder.BeginSegment("lambda", TraceId);
#if NET45
lambda.Invoke(new InvokeRequest
Expand Down Expand Up @@ -325,7 +327,7 @@ public void TestRegisterXRayManifestWithStreamLambdaForAWSSDKHandler()
AWSSDKHandler.RegisterXRayManifest(stream);
}
var lambda = new AmazonLambdaClient(new AnonymousAWSCredentials(), RegionEndpoint.USEast1);
CustomResponses.SetResponse(lambda, null, null, true);
CustomResponses.SetResponse(lambda, null, null, null, true);
_recorder.BeginSegment("lambda", TraceId);
#if NET45
lambda.Invoke(new InvokeRequest
Expand All @@ -343,5 +345,32 @@ public void TestRegisterXRayManifestWithStreamLambdaForAWSSDKHandler()

Assert.AreEqual("Invoke", segment.Subsegments[0].Aws["operation"]);
}

[TestMethod]
public void TestSNSSubsegment()
{
AWSSDKHandler.RegisterXRay<IAmazonSimpleNotificationService>();
var sns = new AmazonSimpleNotificationServiceClient(new AnonymousAWSCredentials(), RegionEndpoint.USEast1);
CustomResponses.SetResponse(sns, null, null, null, true);

_recorder.BeginSegment("test sns", TraceId);

try
{
sns.ListTopicsAsync().Wait();
}
catch
{
// will throw exception for using anonymous AWS credentials, but will not affect generating traces
}

var segment = _recorder.TraceContext.GetEntity();
_recorder.EndSegment();
Assert.AreEqual(1, segment.Subsegments.Count);
Assert.AreEqual("SNS", segment.Subsegments[0].Name); // Name should be SNS instead of SimpleNotificationService
Assert.AreEqual("ListTopics", segment.Subsegments[0].Aws["operation"]);
Assert.AreEqual("us-east-1", segment.Subsegments[0].Aws["region"]);
Assert.AreEqual("aws", segment.Subsegments[0].Namespace);
}
}
}
1 change: 1 addition & 0 deletions sdk/test/UnitTests/AWSXRayRecorder.UnitTests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
<PackageReference Include="AWSSDK.DynamoDBv2" Version="3.5.1.1" />
<PackageReference Include="AWSSDK.Lambda" Version="3.5.0.25" />
<PackageReference Include="AWSSDK.S3" Version="3.5.3.1" />
<PackageReference Include="AWSSDK.SimpleNotificationService" Version="3.5.0.26" />
<PackageReference Include="Moq" Version="4.7.137" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp2.0'">
Expand Down
26 changes: 20 additions & 6 deletions sdk/test/UnitTests/Tools/CustomResponses.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ public static class CustomResponses
{
#if NET45
public static void SetResponse(
AmazonServiceClient client, string content, string requestId, bool isOK)
AmazonServiceClient client, string content, string requestId, string s3ExtendedRequestId, bool isOK)
lupengamzn marked this conversation as resolved.
Show resolved Hide resolved
{
var response = Create(content, requestId, isOK);
var response = Create(content, requestId, s3ExtendedRequestId, isOK);
SetResponse(client, response);
}

Expand All @@ -54,7 +54,7 @@ public static void SetResponse(
}

private static Func<MockHttpRequest, HttpWebResponse> Create(
string content, string requestId, bool isOK)
string content, string requestId, string s3ExtendedRequestId, bool isOK)
{
var status = isOK ? HttpStatusCode.OK : HttpStatusCode.NotFound;

Expand All @@ -65,6 +65,13 @@ private static Func<MockHttpRequest, HttpWebResponse> Create(
{
headers.Add(HeaderKeys.RequestIdHeader, requestId);
}

// https://aws.amazon.com/premiumsupport/knowledge-center/s3-request-id-values/
if (!string.IsNullOrEmpty(s3ExtendedRequestId))
{
headers.Add(HeaderKeys.XAmzId2Header, s3ExtendedRequestId);
}

var response = MockWebResponse.Create(status, headers, content);

if (isOK)
Expand All @@ -77,9 +84,9 @@ private static Func<MockHttpRequest, HttpWebResponse> Create(
}
#else

public static void SetResponse(AmazonServiceClient client, string content, string requestId, bool isOK)
public static void SetResponse(AmazonServiceClient client, string content, string requestId, string s3ExtendedRequestId, bool isOK)
{
var response = Create(content, requestId, isOK);
var response = Create(content, requestId, s3ExtendedRequestId, isOK);
SetResponse(client, response);
}

Expand All @@ -98,7 +105,7 @@ public static void SetResponse(AmazonServiceClient client, Func<MockHttpRequest,
}

private static Func<MockHttpRequest, HttpResponseMessage> Create(
string content, string requestId, bool isOK)
string content, string requestId, string s3ExtendedRequestId, bool isOK)
{
var status = isOK ? HttpStatusCode.OK : HttpStatusCode.NotFound;

Expand All @@ -109,6 +116,13 @@ private static Func<MockHttpRequest, HttpResponseMessage> Create(
{
headers.Add(HeaderKeys.RequestIdHeader, requestId);
}

// https://aws.amazon.com/premiumsupport/knowledge-center/s3-request-id-values/
if (!string.IsNullOrEmpty(s3ExtendedRequestId))
{
headers.Add(HeaderKeys.XAmzId2Header, s3ExtendedRequestId);
}

var response = MockWebResponse.Create(status, headers, content);

if (isOK)
Expand Down