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.

Sign up for GitHub

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

Fixing AWS SDK handler for registering all clients using custom AWS Service manifest file #75

Merged
merged 4 commits into from
Apr 23, 2019
Merged

Fixing AWS SDK handler for registering all clients using custom AWS Service manifest file #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d0e470e
Merge pull request #1 from aws/master
Mar 29, 2019
de9ff05
Merge pull request #2 from aws/master
Apr 16, 2019
0e3f8aa
Merge pull request #3 from aws/master
Apr 23, 2019
c918989
Fixing AWS SDK handler for registering all clients using custom AWS S…
Apr 23, 2019
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
10 changes: 5 additions & 5 deletions sdk/src/Handlers/AwsSdk/AWSSDKHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public static void RegisterXRayForAllServices(String path)
{
_customizer = GetCustomizer();
_customizer.RegisterAll = true;
_customizer.XRayPipelineHandler = new XRayPipelineHandler(path);
_customizer.AWSServiceHandlerManifest = XRayPipelineHandler.GetAWSServiceManifest(path);
}

/// <summary>
Expand All @@ -63,8 +63,8 @@ public static void RegisterXRay<T>()
/// <param name="path"> Absolute path to the file which contains the operation parameter whitelist configuration.</param>
public static void RegisterXRayManifest(String path)
{
_customizer = GetCustomizer();
_customizer.XRayPipelineHandler = new XRayPipelineHandler(path);
_customizer = GetCustomizer();
_customizer.AWSServiceHandlerManifest = XRayPipelineHandler.GetAWSServiceManifest(path);
}

/// <summary>
Expand All @@ -73,8 +73,8 @@ public static void RegisterXRayManifest(String path)
/// <param name="stream"> stream for manifest which contains the operation parameter whitelist configuration.</param>
public static void RegisterXRayManifest(Stream stream)
{
_customizer = GetCustomizer();
_customizer.XRayPipelineHandler = new XRayPipelineHandler(stream);
_customizer = GetCustomizer();
_customizer.AWSServiceHandlerManifest = XRayPipelineHandler.GetAWSServiceManifest(stream);
}

private static XRayPipelineCustomizer GetCustomizer()
Expand Down
102 changes: 70 additions & 32 deletions sdk/src/Handlers/AwsSdk/Internal/XRayPipelineHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public class XRayPipelineHandler : PipelineHandler
public XRayPipelineHandler()
{
_recorder = AWSXRayRecorder.Instance;
InitWithDefaultAWSWhitelist(_recorder);
AWSServiceHandlerManifest = GetDefaultAWSWhitelist();
}

/// <summary>
Expand All @@ -76,20 +76,10 @@ public XRayPipelineHandler(string path)
throw new ArgumentNullException("recorder");
}

if (string.IsNullOrEmpty(path))
{
_logger.DebugFormat("The path is null or empty, initializing with default AWS whitelist.");
InitWithDefaultAWSWhitelist(_recorder);
}
else
{
using (Stream stream = new FileStream(path, FileMode.Open, FileAccess.Read))
{
Init(_recorder, stream);
}
}
AWSServiceHandlerManifest = GetAWSServiceManifest(path);
}


/// <summary>
/// Initializes a new instance of the <see cref="XRayPipelineHandler" /> class.
/// </summary>
Expand All @@ -104,17 +94,65 @@ public XRayPipelineHandler(Stream stream)
throw new ArgumentNullException("recorder");
}

if (stream == null)
AWSServiceHandlerManifest = GetAWSServiceManifest(stream);
}

/// <summary>
/// Creates instance of <see cref="XRayPipelineHandler"/> with provided AWS service manifest instance.
/// </summary>
/// <param name="awsServiceManifest">Instance of <see cref="AWSServiceHandlerManifest"/></param>
public XRayPipelineHandler(AWSServiceHandlerManifest awsServiceManifest)
{
_recorder = AWSXRayRecorder.Instance;

if (_recorder == null)
{
_logger.DebugFormat("The provided stream is null, initializing with default AWS whitelist.");
InitWithDefaultAWSWhitelist(_recorder);
throw new ArgumentNullException("recorder");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like if the recorder is missing when the XRayPipelineHandler is instantiated we'll throw an exception that isn't caught. Is this the same behavior as before the custom manifest was supported, or should it be logged instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new constructor and inline with existing constructor which maintains the same behavior.

}

AWSServiceHandlerManifest = awsServiceManifest;
}

/// <summary>
/// Extracts <see cref="AWSServiceHandlerManifest"/> instance from provided path of AWS Service manifest file.
/// </summary>
/// <param name="path">Absolute path to AWS Service Manifest file</param>
/// <returns>Instance of <see cref="AWSServiceHandlerManifest"/></returns>
public static AWSServiceHandlerManifest GetAWSServiceManifest(string path)
{
if (string.IsNullOrEmpty(path))
{
_logger.DebugFormat("The path is null or empty, initializing with default AWS whitelist.");
return GetDefaultAWSWhitelist();
}
else
{
using (Stream stream = new FileStream(path, FileMode.Open, FileAccess.Read))
{
return GetAWSServiceHandlerManifest(stream);
}
}
}

/// <summary>
/// Extracts <see cref="AWSServiceHandlerManifest"/> instance from provided aws service manifest stream.
/// </summary>
/// <param name="stream">Absolute path to AWS Service Manifest file</param>
/// <returns>Instance of <see cref="AWSServiceHandlerManifest"/></returns>
public static AWSServiceHandlerManifest GetAWSServiceManifest(Stream stream)
{
if (stream == null)
{
_logger.DebugFormat("The provided stream is null, initializing with default AWS whitelist.");
return GetDefaultAWSWhitelist();
}
else
{
Init(_recorder, stream);
}
}

return GetAWSServiceHandlerManifest(stream);
}
}


private static bool TryReadPropertyValue(object obj, string propertyName, out object value)
{
value = 0;
Expand Down Expand Up @@ -241,28 +279,27 @@ private static void AddListLengthProperty(IDictionary<string, object> aws, objec
aws[newPropertyName.FromCamelCaseToSnakeCase()] = listValue.Count;
}

private void InitWithDefaultAWSWhitelist(AWSXRayRecorder recorder)
private static AWSServiceHandlerManifest GetDefaultAWSWhitelist()
{
using (var stream = Assembly.GetExecutingAssembly().GetManifestResourceStream(DefaultAwsWhitelistManifestResourceName))
{
Init(recorder, stream);
return GetAWSServiceHandlerManifest(stream);
}
}

private void Init(AWSXRayRecorder recorder, Stream stream)
private static AWSServiceHandlerManifest GetAWSServiceHandlerManifest(Stream stream)
{
_recorder = recorder;

using (var reader = new StreamReader(stream))
{
try
{
AWSServiceHandlerManifest = JsonMapper.ToObject<AWSServiceHandlerManifest>(reader);
return JsonMapper.ToObject<AWSServiceHandlerManifest>(reader);
}
catch (JsonException e)
{
_logger.Error(e, "Failed to load AWSServiceHandlerManifest.");
}
return null;
}
}

Expand Down Expand Up @@ -674,10 +711,11 @@ public class XRayPipelineCustomizer : IRuntimePipelineCustomizer
private Boolean registerAll;
private List<Type> types = new List<Type>();
private ReaderWriterLockSlim rwLock = new ReaderWriterLockSlim();

public bool RegisterAll { get => registerAll; set => registerAll = value; }
public string Path { get; set; }
public XRayPipelineHandler XRayPipelineHandler { get; set; } = null;
public string Path { get; set; } // TODO :: This is not used anymore, remove in next breaking change
public XRayPipelineHandler XRayPipelineHandler { get; set; } = null; // TODO :: This is not used anymore, remove in next breaking change
public AWSServiceHandlerManifest AWSServiceHandlerManifest { get; set; } = null;

public void Customize(Type serviceClientType, RuntimePipeline pipeline)
{
Expand All @@ -691,13 +729,13 @@ public void Customize(Type serviceClientType, RuntimePipeline pipeline)
addCustomization = ProcessType(serviceClientType, addCustomization);
}

if (addCustomization && XRayPipelineHandler == null)
if (addCustomization && AWSServiceHandlerManifest == null)
{
pipeline.AddHandlerAfter<EndpointResolver>(new XRayPipelineHandler());
}
else if (addCustomization && XRayPipelineHandler != null)
else if (addCustomization && AWSServiceHandlerManifest != null)
{
pipeline.AddHandlerAfter<EndpointResolver>(XRayPipelineHandler); // Custom AWS Manifest file path provided
pipeline.AddHandlerAfter<EndpointResolver>(new XRayPipelineHandler(AWSServiceHandlerManifest)); // Custom AWS Manifest file path/stream provided
}
}

Expand Down