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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update AWS-SDK packages #1742

Merged
merged 10 commits into from
Sep 15, 2021
Merged

Update AWS-SDK packages #1742

merged 10 commits into from
Sep 15, 2021

Conversation

kr8n3r
Copy link
Contributor

@kr8n3r kr8n3r commented Aug 12, 2021

What

@aws-sdk/client-cloudwatch-node and @aws-sdk/client-resource-groups-tagging-api-node have been deprecated and replaced with @aws-sdk/client-cloudwatch and @aws-sdk/client-resource-groups-tagging-api respectively

This updated the packages and imports where required

Modular AWS SDK for JavaScript

How to review

  • deploy to a dev env
  • check affected metrics have data

Who can review

not @kr8n3r


馃毃鈿狅笍 Please do not merge this pull request via the GitHub UI 鈿狅笍馃毃

@kr8n3r
Copy link
Contributor Author

kr8n3r commented Aug 17, 2021

turns out we have a v2 of the sdk and these two packages are v3, so there might be an API or payload discrepancy

@paroxp paroxp force-pushed the update-aws-packages branch 2 times, most recently from 56b7e02 to 22093ac Compare September 8, 2021 12:48
@kr8n3r
Copy link
Contributor Author

kr8n3r commented Sep 8, 2021

I see what you did here.

I was hoping that we could do a deep compare fo two objects with

expect(send.mock.calls[0][0]).toEqual(expectedArgs); where expectedArgs is for example

 const expectedArgs = new GetResourcesCommand({
    ResourceTypeFilters: ['cloudfront:distribution'],
    TagFilters: [
      {
        Key: 'ServiceInstance',
        Values: ['a-service-guid'],
      },
    ],
  }) 

but the comparison still fails with ( Received: serializes to the same string) so I think this stringify extend is the best option

note: i cannot approve my own PR :)

@kr8n3r kr8n3r force-pushed the update-aws-packages branch 3 times, most recently from 468919c to 2971fa1 Compare September 13, 2021 08:52
@kr8n3r
Copy link
Contributor Author

kr8n3r commented Sep 13, 2021

so stricter date parsing has been introduced in
3.30
seems like they want * Input strings must conform to RFC3339 section 5.6, and cannot have a UTC * offset. Fractional precision is supported. so
our tests are now failing with ``` TypeError: Invalid RFC-3339
date-time value at parseRfc3339DateTime
(node_modules/@aws-sdk/smithy-client/src/date-utils.ts:66:11)
at node_modules/@aws-sdk/client-cloudwatch/protocols/Aws_query.ts:6321:52
at Array.map ()
at deserializeAws_queryTimestamps (node_modules/@aws-sdk/client-cloudwatch/protocols/Aws_query.ts:6317:6)
at deserializeAws_queryMetricDataResult (node_modules/@aws-sdk/client-cloudwatch/protocols/Aws_query.ts:5991:27)
at node_modules/@aws-sdk/client-cloudwatch/protocols/Aws_query.ts:6035:14
at Array.map ()
at deserializeAws_queryMetricDataResults (node_modules/@aws-sdk/client-cloudwatch/protocols/Aws_query.ts:6031:6)
at deserializeAws_queryGetMetricDataOutput (node_modules/@aws-sdk/client-cloudwatch/protocols/Aws_query.ts:5284:34)
at deserializeAws_queryGetMetricDataCommand (node_modules/@aws-sdk/client-cloudwatch/protocols/Aws_query.ts:1756:14)

cloudwatch, even thoug log of `rangeStart` and `rangeStop` at
https://github.com/alphagov/paas-admin/blob/main/src/components/service-metrics/controllers.tsx#L211
returns `2021-09-13T12:17:00.000Z` `2021-09-13T13:17:00.000Z`

UPDATE: new date parsing for cloudwatch was done on the output data not input.
Our stub data was in unix timestamp string so it needed to be converted to 'yyyy-MM-ddTHH:mm:ssZ format

@kr8n3r kr8n3r force-pushed the update-aws-packages branch 2 times, most recently from ac91c9a to 98c520f Compare September 14, 2021 07:53
kr8n3r and others added 10 commits September 15, 2021 12:28
`@aws-sdk/client-cloudwatch-node` and
`@aws-sdk/client-resource-groups-tagging-api-node`
have been deprecated and replaced with `@aws-sdk/client-cloudwatch`
and `@aws-sdk/client-resource-groups-tagging-api`respectively

additionally add aws-sdk types for v3
In my belief, these tests have been failing, as the function comparing
two inputs is simply comparing two different yet identical objects. It's
similar to:

```js
console.log({} === {});
```

Each of those two identical objects, take a different place in memory
allocation, making them different.

Instead, we're extending the `jest` functionality, to allow us to
compare dumb down version of these objects, in its purest stringified
form, essentially comparing the two strings together as oppose to memory
allocations.

```js
console.log(JSON.stringify({}) === JSON.stringify({}));
```

What's confusing to me really, is how this has worked before. And in
reality, this is not a fault with the AWS library itself, but rather
with `jest` comparisions.

But "at this point, I'm too afraid to ask" and instead of performing
more insightful investigation, I'm just going to get on with my life.
Running everything at once is perhaps useful on a local machine, but is
also very noisy.

In its current confugiration, the build step is being run twice.

Separating these jobs, should make things easier to look at and
determine what went wrong and where are the improvements required.
There is a possibility that the series will poses some invalid dates. We
don't want to be dealing with that, as it may produce some bad outcome
for the end user, where for instance, a lot of events happened on the
Thursday, 1 January 1970 00:00:00, and then few or none events on
current timing people actually care about.

Arguably, filtering them out could leave the graph empty, but at this
point I feel like there other issues we have on hands as oppose to
irrational filtering out?
In release 3.30.0 (aws/aws-sdk-js-v3#2737)
AWS-SDK put stricter timestamp parsing.

Our stub data for cloudwatch metrics was outputing unix timestamp
strings, so aws-sdk parser was throwing `TypeError: Invalid RFC-3339
date-time`

In 7b1edc2
we updated the `getGappyRandomData` function to return unix timestamp
to address Prometheus stub data formats.

We don't want to change the output of the `getGappyRandomData` function,
so for cloudwatch stub data, we'll:

- parse the unixtimestamp string to an integer
- convert it to a valid date
- format date to RFC-3339 (ISO 8601-ish) format

Now AWS-SDK parser is happy.
@kr8n3r kr8n3r merged commit 42f36a6 into main Sep 15, 2021
@kr8n3r kr8n3r deleted the update-aws-packages branch September 15, 2021 11:38
@kr8n3r kr8n3r mentioned this pull request Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants