Applied HttpClient to get sampling info #159

lupengamzn · 2020-11-04T08:50:37Z

Description of changes:

Applied HttpWebRequest to get sampling rules and targets
Removed dependency on AWSSDK.XRay

Targeting: #150 #151 #152 #153

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

… AWSSDK.XRay

sdk/src/Core/Sampling/ServiceConnector.cs

srprash

Mostly looks good! Some doc related changes and a question. :)

sdk/src/Core/Sampling/GetSamplingRulesResponse.cs

sdk/src/Core/Sampling/GetSamplingTargetsResponse.cs

sdk/src/Core/Sampling/XRayConfig.cs

srprash · 2020-11-05T18:15:41Z

sdk/test/UnitTests/JsonMapperTest.cs

+
+            samplingStatisticsModel.SamplingStatisticsDocuments.Add(samplingStatisticsDocumentModel);
+
+            var expected = "{\"SamplingStatisticsDocuments\":[{\"ClientID\":null,\"RuleName\":null,\"RequestCount\":null,\"SampledCount\":null,\"BorrowCount\":null,\"Timestamp\":null}]}";


What's the response from the GetSamplingTargets API if the sampling statistics document has null/empty required fields? Do we need to handle any exceptions in that case?

No, they didn't do sanity check for the fields. All of the sampling statistics document fields can be nullable and passing a sampling statistics document object with empty values to GetSamplingTargets API will not throw exception.

lupengamzn · 2020-11-06T18:32:38Z

@srprash No, ExpectContinue is Nullable, its default value is null. When using POST method with content to get sampling targets in .NET framework, there will be an Expect header in request which will cause signature invalid error from X-Ray backend. We have to disable ExpectContinue header similar as what AWS SDK does to avoid this issue.

srprash

Looks good! :)

* Create stale.yml (#136) * Release commit for v2.9.0 (#137) * Added 2 seconds timeout for EC2 metadata requests (#138) * Update NOTICE so it only has third party licenses and no proprietary notice (#139) * Fixing link to EFCore usage section (#143) * Added EF Core installation instruction step (#145) * Fixing race condition for initializing and running timer in RulePoller (#154) * Bump AWSSDK.Core dependency version (#155) * Bump AWSSDK.Core dependency version * Bumped AWSSDK.Core version as range. * Removed redundant dependency * Applied HttpClient to get sampling info (#159) * Applied HttpWebRequest to get sampling info and removed dependency on AWSSDK.XRay * Fixed lock issue and switched to HttpClient * Added and modified documentations. * Added documentations. * Set ExpectContinue header as false by default * Adding limitation for EFCore with multiple DbCommandInterceptor (#164) * Adding limitation for EFCore with multiple DbCommandInterceptor * Adding doc ref for AsyncLocal * Updated xraycontext to use a concurrent dictionary (#157) To avoid concurrent thread access to the dictionary causing an exception * Enable tracing S3 id pairs and fix SNS naming issue. (#168) * Enable tracing S3 id pairs and fix SNS naming issue. * Changed AWSSDK.Core version * Changed parameter names and updated comments. * Set parameters as default * Fixed appvoyer building configuration issue. (#173) * Add tracing support for EntityFramework 6 (.NET Framework) (#171) * Added support for EntityFramework 6 for .NET framework. * Added release workflow (#179) * Added release workflow * Added space line * Update configuration * Switching out AppVeyor for GH workflow (#181) * Switching out AppVeyor for GH workflow * Running workflow on linux as well. building wth release config * Revert "Switching out AppVeyor for GH workflow (#181)" (#185) This reverts commit 34134f0. * Added public key token (#186) * Release commit for v2.10.0 (#187) * Updated API Doc (#188) * Add smoke test of distribution channel (#189) * Add smoke test of distribution channel * Updated AWSXRayRecorder.sln * Points to the latest version * Publish metric on distribution availability (#190) * Run test on ubuntu (#191) * Bump System.Net.Http from 4.3.3 to 4.3.4 in /sdk/src/Core (#193) Bumps System.Net.Http from 4.3.3 to 4.3.4. Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Release commit for v2.10.1 (#194) * Update continuous-monitoring.yml * Increase continuous monitoring to 10 minutes * Add netcoreapp3.1 target framework Add a target framework moniker for .NET Core 3.1. * Create CODEOWNERS * Update appveyor.yml (#226) * Update appveyor.yml * Update appveyor.yml * Update .netappcore version for smoke tests (#228) * Update continuous-monitoring.yml * Update netcoreapp version in .csproj file * Update continuous-monitoring.yml * Update AWSXRayRecorder.SmokeTests.csproj * Use GitHub workflow for CI on PRs and master builds (#231) * Update continuous-monitoring.yml * Add Source Link support for enhanced debugging experience (#229) * Add Source Link support for enhanced debugging experience * NetFramework does not support embedded pdb Co-authored-by: Jon Armen <jarmen@bstglobal.com> * Finding the same exception in different subsegments should reuse the same cause id (#210) Co-authored-by: Jon Armen <jarmen@bstglobal.com> * Removed blocking call which was producing deadlocks from EC2Plugin. (#234) * Adding 3.x for continous build Co-authored-by: srprash <50466688+srprash@users.noreply.github.com> Co-authored-by: Anuraag Agrawal <aanuraag@amazon.co.jp> Co-authored-by: Lu Peng <61207760+lupengamzn@users.noreply.github.com> Co-authored-by: Stephen D'Olier <stephen@dolier.net> Co-authored-by: Bhautik Pipaliya <56270044+bhautikpip@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: William Armiros <54150514+willarmiros@users.noreply.github.com> Co-authored-by: martincostello <martin@martincostello.com> Co-authored-by: Jon Armen <jarmen@bstglobal.com> Co-authored-by: Scott Munro <scott.munro@yahoo.com>

* Create stale.yml (aws#136) * Release commit for v2.9.0 (aws#137) * Added 2 seconds timeout for EC2 metadata requests (aws#138) * Update NOTICE so it only has third party licenses and no proprietary notice (aws#139) * Fixing link to EFCore usage section (aws#143) * Added EF Core installation instruction step (aws#145) * Fixing race condition for initializing and running timer in RulePoller (aws#154) * Bump AWSSDK.Core dependency version (aws#155) * Bump AWSSDK.Core dependency version * Bumped AWSSDK.Core version as range. * Removed redundant dependency * Applied HttpClient to get sampling info (aws#159) * Applied HttpWebRequest to get sampling info and removed dependency on AWSSDK.XRay * Fixed lock issue and switched to HttpClient * Added and modified documentations. * Added documentations. * Set ExpectContinue header as false by default * Adding limitation for EFCore with multiple DbCommandInterceptor (aws#164) * Adding limitation for EFCore with multiple DbCommandInterceptor * Adding doc ref for AsyncLocal * Updated xraycontext to use a concurrent dictionary (aws#157) To avoid concurrent thread access to the dictionary causing an exception * Enable tracing S3 id pairs and fix SNS naming issue. (aws#168) * Enable tracing S3 id pairs and fix SNS naming issue. * Changed AWSSDK.Core version * Changed parameter names and updated comments. * Set parameters as default * Fixed appvoyer building configuration issue. (aws#173) * Add tracing support for EntityFramework 6 (.NET Framework) (aws#171) * Added support for EntityFramework 6 for .NET framework. * Added release workflow (aws#179) * Added release workflow * Added space line * Update configuration * Switching out AppVeyor for GH workflow (aws#181) * Switching out AppVeyor for GH workflow * Running workflow on linux as well. building wth release config * Revert "Switching out AppVeyor for GH workflow (aws#181)" (aws#185) This reverts commit 34134f0. * Added public key token (aws#186) * Release commit for v2.10.0 (aws#187) * Updated API Doc (aws#188) * Add smoke test of distribution channel (aws#189) * Add smoke test of distribution channel * Updated AWSXRayRecorder.sln * Points to the latest version * Publish metric on distribution availability (aws#190) * Run test on ubuntu (aws#191) * Bump System.Net.Http from 4.3.3 to 4.3.4 in /sdk/src/Core (aws#193) Bumps System.Net.Http from 4.3.3 to 4.3.4. Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Release commit for v2.10.1 (aws#194) * Update continuous-monitoring.yml * Increase continuous monitoring to 10 minutes * Add netcoreapp3.1 target framework Add a target framework moniker for .NET Core 3.1. * Create CODEOWNERS * Update appveyor.yml (aws#226) * Update appveyor.yml * Update appveyor.yml * Update .netappcore version for smoke tests (aws#228) * Update continuous-monitoring.yml * Update netcoreapp version in .csproj file * Update continuous-monitoring.yml * Update AWSXRayRecorder.SmokeTests.csproj * Use GitHub workflow for CI on PRs and master builds (aws#231) * Update continuous-monitoring.yml * Add Source Link support for enhanced debugging experience (aws#229) * Add Source Link support for enhanced debugging experience * NetFramework does not support embedded pdb Co-authored-by: Jon Armen <jarmen@bstglobal.com> * Finding the same exception in different subsegments should reuse the same cause id (aws#210) Co-authored-by: Jon Armen <jarmen@bstglobal.com> * Removed blocking call which was producing deadlocks from EC2Plugin. (aws#234) * Adding 3.x for continous build Co-authored-by: srprash <50466688+srprash@users.noreply.github.com> Co-authored-by: Anuraag Agrawal <aanuraag@amazon.co.jp> Co-authored-by: Lu Peng <61207760+lupengamzn@users.noreply.github.com> Co-authored-by: Stephen D'Olier <stephen@dolier.net> Co-authored-by: Bhautik Pipaliya <56270044+bhautikpip@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: William Armiros <54150514+willarmiros@users.noreply.github.com> Co-authored-by: martincostello <martin@martincostello.com> Co-authored-by: Jon Armen <jarmen@bstglobal.com> Co-authored-by: Scott Munro <scott.munro@yahoo.com>

Applied HttpWebRequest to get sampling info and removed dependency on…

eebf640

… AWSSDK.XRay

lupengamzn requested review from normj and srprash November 4, 2020 08:50

normj reviewed Nov 5, 2020

View reviewed changes

sdk/src/Core/Sampling/ServiceConnector.cs Show resolved Hide resolved

sdk/src/Core/Sampling/ServiceConnector.cs Outdated Show resolved Hide resolved

sdk/src/Core/Sampling/ServiceConnector.cs Outdated Show resolved Hide resolved

Fixed lock issue and switched to HttpClient

34b0146

lupengamzn requested a review from normj November 5, 2020 10:13

srprash reviewed Nov 5, 2020

View reviewed changes

lupengamzn added 2 commits November 5, 2020 11:02

Added and modified documentations.

1ca5248

Added documentations.

4e1eb2b

lupengamzn requested a review from srprash November 5, 2020 19:11

Set ExpectContinue header as false by default

67e98d7

srprash approved these changes Nov 6, 2020

View reviewed changes

lupengamzn mentioned this pull request Nov 6, 2020

Update AWSSDK.Core version #147

Closed

lupengamzn changed the title ~~Applied HttpWebRequest to get sampling info~~ Applied HttpClient to get sampling info Nov 6, 2020

lupengamzn merged commit a085520 into aws:master Nov 6, 2020

lupengamzn mentioned this pull request Nov 20, 2020

Remove AWSXRayRecorder.Core's Dependency on AWSSDK.Core aws/aws-xray-dotnet-agent#13

Open

dependabot bot mentioned this pull request Mar 7, 2021

Bump AWSXRayRecorder.Core from 2.9.0 to 2.10.0 esilean/aws-project#8

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Applied HttpClient to get sampling info #159

Applied HttpClient to get sampling info #159

lupengamzn commented Nov 4, 2020 •

edited

Loading

srprash left a comment

srprash Nov 5, 2020

lupengamzn Nov 5, 2020

lupengamzn commented Nov 6, 2020

srprash left a comment


		samplingStatisticsModel.SamplingStatisticsDocuments.Add(samplingStatisticsDocumentModel);

		var expected = "{\"SamplingStatisticsDocuments\":[{\"ClientID\":null,\"RuleName\":null,\"RequestCount\":null,\"SampledCount\":null,\"BorrowCount\":null,\"Timestamp\":null}]}";

Applied HttpClient to get sampling info #159

Applied HttpClient to get sampling info #159

Conversation

lupengamzn commented Nov 4, 2020 • edited Loading

srprash left a comment

Choose a reason for hiding this comment

srprash Nov 5, 2020

Choose a reason for hiding this comment

lupengamzn Nov 5, 2020

Choose a reason for hiding this comment

lupengamzn commented Nov 6, 2020

srprash left a comment

Choose a reason for hiding this comment

lupengamzn commented Nov 4, 2020 •

edited

Loading