feat(nrawssdk): Entity Relationship AWS update#1120
feat(nrawssdk): Entity Relationship AWS update#1120cade-conklin wants to merge 9 commits intodevelopfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1120 +/- ##
===========================================
- Coverage 79.08% 78.99% -0.10%
===========================================
Files 159 159
Lines 14935 15019 +84
===========================================
+ Hits 11812 11864 +52
- Misses 2711 2737 +26
- Partials 412 418 +6 see 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
c24338f to
561da80
Compare
| MaxSamplesStored int | ||
| } | ||
|
|
||
| CloudAWS struct { |
There was a problem hiding this comment.
We should notate why we have this config option with a good comment
| *apiOptions = append(*apiOptions, m.serializeMiddleware) | ||
| } | ||
|
|
||
| func NRAppendMiddlewares(apiOptions *[]func(*smithymiddle.Stack) error, ctx context.Context, awsConfig aws.Config) { |
There was a problem hiding this comment.
Are we differentiating our AppendMiddlewares vs. NRAppendMiddlewares enough here? What purpose does the older one still serve & is it something we can remove/combine?
There was a problem hiding this comment.
I should leave more comments here, but I put in a different function since it takes in different parameters! I can discuss the purpose/parameters passed into this function since I think its a bit clunky as it stands
There was a problem hiding this comment.
Note from mobbing: can we have the old function just call this new function?
There was a problem hiding this comment.
Note from mobbing: can we just pass in one awsConfig aws.Config instead of aws.Config.apiOptions + aws.Config?
There was a problem hiding this comment.
@mirackara I am keeping it the same way because we still keep the option to inject our middleware into specific client requests. The mismatching typing for the structs contains apiOptions from the client and the awsConfig prevent a good way to just pass one type in
|
|
||
| mask := uint64(0x7fffffffff80) | ||
|
|
||
| num := (bigEndian & mask) >> 7 // apply mask and get rid of last 7 bytes from mask |
There was a problem hiding this comment.
Yes, thank you!
0ef89ed to
ef3787c
Compare
ef3787c to
5e67221
Compare
Links
Details
Covered in this PR
Entity Relationships for Elastic/Open Search
Entity Relationships, AWS account information config
NEW_RELIC_CLOUD_AWS_ACCOUNT_IDas env varConfig.CloudAWS.AccountIDandConfig.CloudAWS.AccountDecoding.Enabledconfigurable via config options (ConfigCloudAWSAccountIDandConfigCloudAWSAccountDecodingEnabledConfig.CloudAWS.AccountDecoding.Enabledenabled by defaultvalidateAWSAccountIDadded to check when accountID is set in env or by config optionsInitializeMiddlewarefunction added tonrawssdk-v2. Contains logic to retrieve credentials from awsConfig and use them to resolve accountID (with Config setting given priority) and then add existing middlewares to be used in AWS requestsAppendMiddlewaresgiven a note that it is deprecated. It will not add the accountID as a span attributeaccountIDadded as a span attribute inserializeMiddlewaresInitializeMiddleware. It is no longer passed in as an optional function, but rather after the awsConfig is resolved. Still an option to passInitializeMiddlewareas an optional function as detailed in comments forInitializeMiddlewaredescription and in comments in exampleEntity Relationships, decoding AWS account ID
accountIDadded as field innrMiddlewarestruct