WiP: build old router #5010
Draft
WiP: build old router #5010
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was incorrectly using `APOLLO_KEY` rather than `TEST_APOLLO_KEY`,
which messes up local dev for most people.
In addition, the test had not been updated when the metric changed name
a while back so was not actually being exorcised on CI.
<!-- start metadata -->
---
**Checklist**
Complete the checklist (and note appropriate exceptions) before the PR
is marked ready-for-review.
- [ ] Changes are compatible[^1]
- [ ] Documentation[^2] completed
- [ ] Performance impact assessed and acceptable
- Tests added and passing[^3]
- [ ] Unit Tests
- [ ] Integration Tests
- [ ] Manual Tests
**Exceptions**
*Note any exceptions here*
**Notes**
[^1]: It may be appropriate to bring upcoming changes to the attention
of other (impacted) groups. Please endeavour to do this before seeking
PR approval. The mechanism for doing this will vary considerably, so use
your judgement as to how and when to do this.
[^2]: Configuration is an important part of many changes. Where
applicable please try to document configuration examples.
[^3]: Tick whichever testing boxes are applicable. If you are adding
Manual Tests, please document the manual testing (extensively) in the
Exceptions.
Co-authored-by: bryn <bryn@apollographql.com>
Co-authored-by: Coenen Benjamin <benjamin.coenen@hotmail.com>
|
@Geal, please consider creating a changeset entry in |
|
CI performance tests
|
The DD exporter does some explicit mapping of attributes and was using a value "apollo.subgraph.name" that the latest versions of the router don't use. The correct choice is "subgraph.name". Update the mapping to reflect this change.
In the same spirit as the existing Apollo Uplink schema automation which exists in this [file]. This will only allow one PR at a time for this branch name, and only commit changes (and update that branch) when there is a change. [file]: https://github.com/apollographql/router/actions/workflows/update_uplink_schema.yml --------- Co-authored-by: Simon Sapin <simon@apollographql.com>
We'll have a GitHub Action do the maintenance on this file, rather than having a test remind us that it's stale. The test results in all PRs (and all downstream users of this repository) suddenly getting failing tests every time our infra-team deploys a new version of the proto, whereas the GitHub Action can be a much more controlled burn. The GitHub action that replaces this is in [#5030], and we will merge this test just as soon as that is validated. [#5030]: #5030
Surprisingly, the most expensive part of running tests in the router is in JSON schema compilation. Every time we validate a configuration, we generate the JSON configuration schema (which is about 31k lines at this point) then compile a JSONSchema structure used to validate the configuration. This computation can be done once per execution without losing anything, as it only depends on the configuration schema, which is generated from the router's code and native plugins.
This prevents any possibility of port clash, and also doesn't risk the
situation where the OS doesn't release the socket in time before it is
reused by the router.
<!-- start metadata -->
---
**Checklist**
Complete the checklist (and note appropriate exceptions) before the PR
is marked ready-for-review.
- [ ] Changes are compatible[^1]
- [ ] Documentation[^2] completed
- [ ] Performance impact assessed and acceptable
- Tests added and passing[^3]
- [ ] Unit Tests
- [ ] Integration Tests
- [ ] Manual Tests
**Exceptions**
*Note any exceptions here*
**Notes**
[^1]: It may be appropriate to bring upcoming changes to the attention
of other (impacted) groups. Please endeavour to do this before seeking
PR approval. The mechanism for doing this will vary considerably, so use
your judgement as to how and when to do this.
[^2]: Configuration is an important part of many changes. Where
applicable please try to document configuration examples.
[^3]: Tick whichever testing boxes are applicable. If you are adding
Manual Tests, please document the manual testing (extensively) in the
Exceptions.
Co-authored-by: bryn <bryn@apollographql.com>
…runs (#5023) As tends to be the case, our CI testing times have been driven up over time. While we currently have broad-stroke measurements on the CI pipeline, we don't yet have per-test insights. This makes it difficult to commit time to specific efforts, or measure their improvement. Further, the ergonomics over using the failed tests interface on CircleCI have been hindered by the lack of test reporting through well-defined testing output artifacts like Junit, popular with most other languages. Rust core hasn't yet promoted its structured `cargo test` output into a stable build which — if we wanted to measure tests by producing anything remotely coercible into JUnit — would mean we'd have to run `+nightly` runs of the tests. Using the `nightly` build of `cargo test` to get almost (but still not) JUnit seems unnecessarily risky. Given that this feature hasn't promoted from `+nightly` in what must be plural years and the allure of other more feature-complete testing tools like [Nextest](https://nexte.st/) — I decided to give Nextest a quick try. The quick test is seemingly successful and validating: It did not show any degradation in test execution time, is already showing promising nice wins in CircleCI interface experience, seems like an improvement to the CLI experience (image below, but also by [highlighting specific test failures in the UI](https://app.circleci.com/pipelines/github/apollographql/router/20805/workflows/09ba269e-b917-4d2e-ad52-68e80e14ab94/jobs/140871/tests)), unlocks other capabilities (like automatic retries on a per-test basis), and didn't require passing custom thread flags ([perhaps due to a better model for test execution](https://nexte.st/book/how-it-works)?).  ### Local vs CI / How to enable nextest locally Nextest is only used if it is installed and the decision to do so is made inside of `cargo xtask test` based on the presence of `cargo-nextest`. If `cargo-nextest` is **not** installed, then the existing `cargo test` is used as a fall-back. On CI, it is installed and thus used.
Updates our notion of `current` to the latest version `windows-server-2019-vs2019:2024.02.21` but _pins_ it, so it doesn't change randomly.
To try and increase confidence that nothing is wrong with Nick's PR. Replacement for #5049 which (for unknown reasons) isn't building cleanly. Fixes a few issues with the router implementation of the signature generation. Mostly related to commas being inserted instead of spaces or vice-versa, but also includes a significant issue where fragments were not being included in the signature. Co-authored-by: [bonnici](https://github.com/bonnici)
Co-authored-by: Geoffroy Couprie <apollo@geoffroycouprie.com> Co-authored-by: Gary Pennington <gary@apollographql.com>
Co-authored-by: bryn <bryn@apollographql.com>
Fix #5003 This reworks the json schema generation for the config so that it is reduced in size from approx 100k lines to jsut over 7k. The fix involves three things: * Enable references on json schema generation. This got disabled in the past because there were issues with the generated references, but by adding a schema visitor we can work around this. * Adjust the schema generation for Extendable and Conditional. These previously relied on the scheme not using references. * Modify orbiter metrics to redact only based on the properties in the schema rather than on validation metadata as this is not possible when using schema refs: Incomplete basic output Stranger6667/jsonschema-rs#403
Co-authored-by: Nick Marsh <nick.marsh@apollographql.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most commits applied cleanly and immediately.
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Description here
Fixes #issue_number
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩ ↩2
Configuration is an important part of many changes. Where applicable please try to document configuration examples. 3: Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩ ↩2
Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩ ↩2 ↩3