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

feat: serve AsyncAPI JSON Schema definitions #680

Merged
merged 19 commits into from
Jun 21, 2022

Conversation

smoya
Copy link
Member

@smoya smoya commented Apr 22, 2022

Description

This PR let's our website to serve all AsyncAPI JSON Schema definition files from it.

All AsyncAPI JSON Schema definition files are being served within the /definitions/<file> path. The content is literally being served from GH, in particular from https://github.com/asyncapi/spec-json-schemas/tree/master/schemas.
This is possible thanks to the following:

  1. A Netlify Rewrite rule located in the netlify.toml file, which acts as proxy for all requests to the /definitions/<file> path, serving the content from GH without having an HTTP redirect.
  2. A Netlify Edge Function that modifies the Content-Type header of the rewrite response to become application/schema+json. This lets tooling, such as Hyperjump, to fetch the schemas directly from their URL.

In order to see what the final result is, just grab the Netlify build preview link and run:

curl -v <preview-url>/definitions/2.3.0.json

Btw, this Netlify edge function could be used also for pushing metrics on the hit count, solving #780.

Related issue(s)
Iteration over #502

cc @jonaslagoni @derberg @fmvilas @magicmatatjahu

@netlify
Copy link

netlify bot commented Apr 22, 2022

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit e76f8f6
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/62ab034265a33300082a4d3f
😎 Deploy Preview https://deploy-preview-680--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Apr 22, 2022

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 32
🟢 Accessibility 98
🟠 Best practices 83
🟢 SEO 100
🔴 PWA 30

Lighthouse ran on https://deploy-preview-680--asyncapi-website.netlify.app/

README.md Outdated Show resolved Hide resolved
Copy link
Member

@quetzalliwrites quetzalliwrites left a comment

Choose a reason for hiding this comment

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

just one tiny suggestion @smoya 🐼

@derberg
Copy link
Member

derberg commented Apr 28, 2022

Interesting approach 🤔

  • I understand that we can have an edge function that on each request can do anything, like API call to some service, right?
  • You do know it is experimental? Also have a look at -> https://github.community/t/raw-githubusercontent-com-rate-limit/142444. For me, it is actually ok because I have a totally opposite view on the High Availability argument you made here. Just want to make sure you are aware
    Screenshot 2022-04-28 at 11 40 02

smoya and others added 3 commits May 2, 2022 15:36
Co-authored-by: Alejandra Quetzalli  <alejandra.quetzalli@postman.com>
@smoya
Copy link
Member Author

smoya commented May 2, 2022

  • I understand that we can have an edge function that on each request can do anything, like API call to some service, right?

Yes, that's right.

  • You do know it is experimental?

Yes I know, but right now it is a public beta which has been passed through the private beta stage.

For me, it is actually ok because I have a totally opposite view on the High Availability argument you made #780.

I still believe Netlify availability is pretty reliable and superior to what we can support ourselves with the people who are involved. Including using beta features such as Edge Functions.

Also have a look at -> https://github.community/t/raw-githubusercontent-com-rate-limit/142444

You made a good point here. The authentication token is going to be needed to have those 5k req/hour.
I could not make proper testing around how github counts a cached VS non-cached request in terms of rate limit because GH API /rate_limit endpoint doesn't shows such stats for raw.githubusercontent.

However, there is something to consider here and it's the fact Netlify should cache each request for "some time":

If the API supports standard HTTP caching mechanisms like ETags or Last-Modified headers, the responses will even get cached by our CDN nodes.
Source: https://docs.netlify.com/routing/redirects/rewrites-proxies/#proxy-to-another-service

If we run the following:

curl -I -H "Authorization: $MY_TOKEN" https://raw.githubusercontent.com/asyncapi/spec/v2.4.0/examples/websocket-gemini.yml

HTTP/2 200
cache-control: max-age=300
content-security-policy: default-src 'none'; style-src 'unsafe-inline'; sandbox
content-type: text/plain; charset=utf-8
etag: "755fbfddca88cfd66072e57e21f9a33f3fabbe6ae24ce7c0249d83ab8d3d7788"
strict-transport-security: max-age=31536000
x-content-type-options: nosniff
x-frame-options: deny
x-xss-protection: 1; mode=block
x-github-request-id: F288:9F7F:6918C6:6DD3EC:626FDD1E
accept-ranges: bytes
date: Mon, 02 May 2022 13:31:10 GMT
via: 1.1 varnish
x-served-by: cache-mad22067-MAD
x-cache: MISS
x-cache-hits: 0
x-timer: S1651498270.022946,VS0,VE205
vary: Authorization,Accept-Encoding,Origin
access-control-allow-origin: *
x-fastly-request-id: 5327fa939e275e7ddbd14bc99a8fb310421282a5
expires: Mon, 02 May 2022 13:36:10 GMT
source-age: 0
content-length: 9100

We can see the content should be cached for 5 mins (300 seconds): cache-control: max-age=300. And considering that Vary header value is Authorization,Accept-Encoding,Origin, we can also do the following:

  • Remove the Origin request header, so GH always serves the same cached content, no matter the Origin header.
  • Regarding the Accept-Encoding, we should not modify it. It is the expected by the client encoding.

That would give us a really big number of requests allowed (cached or non cached), all pending to be tested and confirmed. In the really worst scenario, we could switch to what @jonaslagoni suggested in #502 so we serve the files right from Netlify instead of GH, but keeping the Edge Function in there for the Headers change + metrics collection.

request.headers.delete("origin");

// Setting GH Token to increase GH rate limit to 5,000 req/h.
request.headers.set("Authorization", "token " + Deno.env.get("GITHUB_TOKEN"));
Copy link
Member Author

Choose a reason for hiding this comment

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

We would need to use a GH token here. Preferably, a token from a user is not being used in any automation yet. cc @derberg

Copy link
Member

Choose a reason for hiding this comment

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

a token from a user

what do you mean? why user token. We can use AsyncAPI token

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, i meant using a new user because requests to GH API are counted across the whole account, no matters wich toke you use. Important for the rate limit.

@smoya
Copy link
Member Author

smoya commented May 4, 2022

I added the following:

Some env vars will be needed to be set at Netlify level:

  • GITHUB_TOKEN: Preferably, a token from a user is not being used in any automation yet.
  • NR_ACCOUNT: the New Relic account id. Can be found on New Relic's website, path: /admin-portal/api-keys.
  • NR_API_KEY: The New Relic API KEY: A License type API Key that can be created on New Relic's website, path: /admin-portal/api-keys

As a side note, the URL to NewRelic insights collector should be changed in case AsyncAPI New Relic Account is not hosted in EU to https://insights-collector.newrelic.com/v1/accounts/YOUR_ACCOUNT_ID/events.

Here a screenshot of how our dashboard could look in New Relic:

2022-05-04 at 15 15 24

@derberg
Copy link
Member

derberg commented May 12, 2022

I think we need some page under https://deploy-preview-680--asyncapi-website.netlify.app/schema-store that explains the concept and lists all available schemas

also to go further lets make sure others have nothing against new relic and hosting from GitHub Raw atm -> #780

@smoya
Copy link
Member Author

smoya commented Jun 10, 2022

I think we need some page under https://deploy-preview-680--asyncapi-website.netlify.app/schema-store that explains the concept and lists all available schemas

Who could help on that part?

also to go further lets make sure others have nothing against new relic and hosting from GitHub Raw atm -> #780

It seems people agreed on this finally!

@smoya
Copy link
Member Author

smoya commented Jun 14, 2022

@derberg redirects have been added to the schemas README from asyncapi/spec-json-schemas#229, which will be located at https://github.com/asyncapi/spec-json-schemas/tree/master/schemas

@smoya
Copy link
Member Author

smoya commented Jun 14, 2022

This is ready for review. Tested locally through netlify dev.

@smoya
Copy link
Member Author

smoya commented Jun 15, 2022

@derberg I checked the retention New Relic provides to the free and standard tier:
2022-06-15 at 17 57 04
Source: https://newrelic.com/pricing

As I was sending custom events, I decided to change it to send metrics instead, so we can take profit of the 13 months of retention. That means the code changed a bit so you will need to review it again (sorry for that).

Also, the NR_ACCOUNT env var is no longer needed with this new approach.

Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

This is really cool 🔝

Everything looks good to me. Do you want to wait for @derberg's review?

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

❤️
Once it is merged we need to open PR to schemastore

@smoya
Copy link
Member Author

smoya commented Jun 15, 2022

❤️ Once it is merged we need to open PR to schemastore

I'm happy to do it

@smoya
Copy link
Member Author

smoya commented Jun 15, 2022

/rtm

@smoya
Copy link
Member Author

smoya commented Jun 15, 2022

/rtm

@smoya
Copy link
Member Author

smoya commented Jun 15, 2022

/help

@asyncapi-bot
Copy link
Contributor

Hello, @smoya! 👋🏼

I'm Genie from the magic lamp. Looks like somebody needs a hand! 🆘

At the moment the following comments are supported in pull requests:

  • /ready-to-merge or /rtm - This comment will trigger automerge of PR in case all required checks are green, approvals in place and do-not-merge label is not added
  • /do-not-merge or /dnm - This comment will block automerging even if all conditions are met and ready-to-merge label is added
  • /autoupdate or /au - This comment will add autoupdate label to the PR and keeps your PR up-to-date to the target branch's future changes. Unless there is a merge conflict or it is a draft PR.

@smoya
Copy link
Member Author

smoya commented Jun 15, 2022

@alequetzalli I need your review again after all suggestions were applied. 🙏 thank you!

@smoya
Copy link
Member Author

smoya commented Jun 16, 2022

❤️ Once it is merged we need to open PR to schemastore

Changes are pushed to my fork but waiting for this PR to be merged (approval by @alequetzalli is required) to be turned into a PR. https://github.com/SchemaStore/schemastore/compare/master...smoya:feat/asyncapiSchemaStore?expand=1

Copy link
Member

@quetzalliwrites quetzalliwrites left a comment

Choose a reason for hiding this comment

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

🤪🥳🫡

@smoya
Copy link
Member Author

smoya commented Jun 21, 2022

/rtm

@asyncapi-bot asyncapi-bot merged commit 60a698b into asyncapi:master Jun 21, 2022
@smoya smoya deleted the feat/definitions branch June 21, 2022 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants