Skip to content

Release 1.5.0#2206

Closed
BrynCooke wants to merge 73 commits intomainfrom
1.5.0
Closed

Release 1.5.0#2206
BrynCooke wants to merge 73 commits intomainfrom
1.5.0

Conversation

@BrynCooke
Copy link
Contributor

Release 1.5.0

abernix and others added 30 commits November 15, 2022 15:02
This updates our dependency on our `apollo-parser` package which brings
a few improvements, including more defensive parsing of some operations.
See its CHANGELOG in [the `apollo-rs`
repository](https://github.com/apollographql/apollo-rs/blob/main/crates/apollo-parser/CHANGELOG.md#032---2022-11-15)
for more details on what's included in the v0.3.2 release.
Merges `main` back to `dev`, albeit a day late.
Here's an example of values.yaml that you could use to mount this to
your container:

```
extraEnvVars:
  - name: APOLLO_ROUTER_SUPERGRAPH_PATH
    value: /data/supergraph-schema.graphql

extraVolumeMounts:
  - name: supergraph-schema
    mountPath: /data
    readOnly: true

extraVolumes:
  - name: supergraph-schema
    configMap:
      name: "{{ .Release.Name }}-supergraph"
      items:
        - key: supergraph-schema.graphql
          path: supergraph-schema.graphql

```

Note: This takes advantage of the fact that we `tpl` template the
extraVolumes in the deployment template, so {{ .Release.Name }} will be
templated into the release name at install. You don't have to do this,
you could just hard-code it, but this is neater.

Here's an example command line:

```
helm upgrade --install --create-namespace --namespace router-test --set-file supergraphFile=supergraph-schema.graphql router-test oci://ghcr.io/apollographql/helm-charts/router --version 1.0.0-rc.9 --values values.yaml
```

NB: rc.9 doesn't exist, so the command is purely illustrative to show
how this works.
…2118)

This PR tries to address Issue #2117

Here's an example of how the Router would now respond when a subgraph
service returns a non-2xx status code and content-type not set to
`application/json`.
```
{
  "data": null,
  "errors": [
    {
      "message": "HTTP fetch failed from 'my-service': 401 Unauthorized",
      "path": [],
      "extensions": {
        "type": "SubrequestHttpError",
        "service": "my-service",
        "reason": "HTTP fetch failed from 'my-service': 401 Unauthorized"
      }
    }
  ]
}
```
fixes: #2095

Most of the example references are still valid, so I've only updated the
ones that raised 404s. I've also fixed a factual error about field
accessibility.
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
fixes: #1932
fixes: #2098 

The next time we release docker images, they will be multi-arch.

Co-authored-by: Jesse Rosenberger <git@jro.cc>
#2116)

### Fix naming inconsistency of
telemetry.metrics.common.attributes.router ([Issue
#2076](#2076))

Mirroring the rest of the config `router` should be `supergraph`

```yaml
telemetry:
  metrics:
    common:
      attributes:
        router: # old
```
becomes
```yaml
telemetry:
  metrics:
    common:
      attributes:
        supergraph: # new
```

### Configuration upgrades ([Issue
#2123](#2123))

Occasionally we will make changes to the Router yaml configuration
format.
When starting the Router if the configuration can be upgraded it will do
so automatically and display a warning:

```
2022-11-22T14:01:46.884897Z  WARN router configuration contains deprecated options: 

  1. telemetry.tracing.trace_config.attributes.router has been renamed to 'supergraph' for consistency

These will become errors in the future. Run `router config upgrade <path_to_router.yaml>` to see a suggested upgraded configuration.
```

Note: If a configuration has errors after upgrading then the
configuration will not be upgraded automatically.

From the CLI users can run:
* `router config upgrade <path_to_router.yaml>` to output configuration
that has been upgraded to match the latest config format.
* `router config upgrade --diff <path_to_router.yaml>` to output a diff
e.g.
```
 telemetry:
   apollo:
     client_name_header: apollographql-client-name
   metrics:
     common:
       attributes:
-        router:
+        supergraph:
           request:
             header:
             - named: "1" # foo
```

There are situations where comments and whitespace are not preserved.
This may be improved in future.

By [@BrynCooke](https://github.com/bryncooke) in
#2116


### CLI structure changes ([Issue
#2123](#2123))

As the Router gains functionality the limitations of the current CLI
structure are becoming apparent.

There is now a separate subcommand for config related operations:
* `config`
  * `schema` - Output the configuration schema
  * `upgrade` - Upgrade the configuration with optional diff support.

`router --schema` has been deprecated and users should move to `router
config schema`.


<!--
First, 🌠 thank you 🌠 for considering a contribution to Apollo!

Some of this information is also included in the /CONTRIBUTING.md file
at the
root of this repository.  We suggest you read it!

  https://github.com/apollographql/router/blob/HEAD/CONTRIBUTING.md

Here are some important details to keep in mind:

* ⏰ Your time is important
To save your precious time, if the contribution you are making will
take more than an hour, please make sure it has been discussed in an
        issue first. This is especially true for feature requests!

* 💡 Features
Feature requests can be created and discussed within a GitHub Issue.
Be sure to search for existing feature requests (and related issues!)
prior to opening a new request. If an existing issue covers the need,
please upvote that issue by using the 👍 emote, rather than opening a
        new issue.

* 🕷 Bug fixes
These can be created and discussed in this repository. When fixing a
bug,
please _try_ to add a test which verifies the fix. If you cannot, you
should
still submit the PR but we may still ask you (and help you!) to create a
test.

* 📖 Contribution guidelines
Follow https://github.com/apollographql/router/blob/HEAD/CONTRIBUTING.md
when submitting a pull request. Make sure existing tests still pass, and
add
        tests for all new behavior.

* ✏️ Explain your pull request
Describe the big picture of your changes here to communicate to what
        your pull request is meant to accomplish. Provide 🔗 links 🔗 to
associated issues! Documentation in the docs/ directory should be
updated
        as necessary.  Finally, a /CHANGELOG.md entry should be added.

We hope you will find this to be a positive experience! Contribution can
be
intimidating and we hope to alleviate that pain as much as possible.
Without
following these guidelines, you may be missing context that can help you
succeed
with your contribution, which is why we encourage discussion first.
Ultimately,
there is no guarantee that we will be able to merge your pull-request,
but by
following these guidelines we can try to avoid disappointment.

-->

Co-authored-by: bryn <bryn@apollographql.com>
fixes #2151

The docs match regex example was wrong, it was missing ^ at the
beginning and $ at the end. `$` marks the end of a string, which could
be a safety issue.
fixes: #2135
fixes: #2145 

This PR repurposes the -debug image to make it the basis for a memory
tracking image which we can use for investigating router memory issues
via heaptrack. (https://github.com/KDE/heaptrack)

The PR is a *breaking* change because it now automatically starts the
router under the control of heaptrack. Technically, it's not really a
breaking change and it's certainly not an API change, but I really want
to draw people's attention to the fact that the debug image will now
execute a lot slower than the non-debug image and use a lot more memory
(to track memory with...).

I've updated the docker documentation to show how to mount a local
directory to store the heaptrack data. I haven't updated the kubernetes
docs, because we don't go into that level of detail and we assume that a
kubernetes devops person would know how to allocate and mount a PVC.
implements retries for subgraph requests. This uses Finagle's retry buckets algorithm
close #1998 

## Basic configuration

By default some logs containing sensible data (like request body,
response body, headers) are not displayed even if we set the right log
level.
For example if you need to display raw responses from one of your
subgraph it won't be displayed by default. To enable them you have to
configure it thanks to the `when_header` setting in the new section
`experimental_logging`. It let's you set different headers to enable
more logs (request/response headers/body for supergraph and subgraphs)
when the request contains these headers with corresponding values/regex.
Here is an example how you can configure it:

```yaml title="router.yaml"
telemetry:
  experimental_logging:
    format: json # By default it's "pretty" if you are in an interactive shell session
    display_filename: true # Display filename where the log is coming from. Default: true
    display_line_number: false # Display line number in the file where the log is coming from. Default: true
    # If one of these headers matches we will log supergraph and subgraphs requests/responses
    when_header:
      - name: apollo-router-log-request
        value: my_client
        headers: true # default: false
        body: true # default: false
      # log request for all requests coming from Iphones
      - name: user-agent
        match: ^Mozilla/5.0 (iPhone*
        headers: true
```

/!\ This PR also upgrade `tracing` (not to the latest version) because I
needed a fix.


Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Fix #2099

to validate the primary part of a request with `@defer`, we reconstruct
a partial query by walking through the query plan. The code responsible
for that was not detecting mutations.

while the PR in the current state fixes the issue, I'd like to refactor
a bit so that we do not special case query and mutation (which will be
annoying if we add subscription at some point), and moving the query
reconstruction elsewhere, to work directly on the query instead of the
query plan
close #2080 

Configuration example:

```yaml
telemetry:
  tracing:
    experimental_response_trace_id:
      enabled: true # default: false
      header_name: "my-trace-id" # default: "apollo-trace-id"
    propagation:
      request: 
        header_name: "x-request-id"
      jaeger: true
```

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Co-authored-by: Geoffroy Couprie <geoffroy@apollographql.com>
Issue was introduced with #2116 but no release had this in.

Move operations would insert data in the config due to the delete magic
value always getting added. Now we check before adding such values.

We may need to move to fluvio-jolt longer term.

<!--
First, 🌠 thank you 🌠 for considering a contribution to Apollo!

Some of this information is also included in the /CONTRIBUTING.md file
at the
root of this repository.  We suggest you read it!

  https://github.com/apollographql/router/blob/HEAD/CONTRIBUTING.md

Here are some important details to keep in mind:

* ⏰ Your time is important
To save your precious time, if the contribution you are making will
take more than an hour, please make sure it has been discussed in an
        issue first. This is especially true for feature requests!

* 💡 Features
Feature requests can be created and discussed within a GitHub Issue.
Be sure to search for existing feature requests (and related issues!)
prior to opening a new request. If an existing issue covers the need,
please upvote that issue by using the 👍 emote, rather than opening a
        new issue.

* 🕷 Bug fixes
These can be created and discussed in this repository. When fixing a
bug,
please _try_ to add a test which verifies the fix. If you cannot, you
should
still submit the PR but we may still ask you (and help you!) to create a
test.

* 📖 Contribution guidelines
Follow https://github.com/apollographql/router/blob/HEAD/CONTRIBUTING.md
when submitting a pull request. Make sure existing tests still pass, and
add
        tests for all new behavior.

* ✏️ Explain your pull request
Describe the big picture of your changes here to communicate to what
        your pull request is meant to accomplish. Provide 🔗 links 🔗 to
associated issues! Documentation in the docs/ directory should be
updated
        as necessary.  Finally, a /CHANGELOG.md entry should be added.

We hope you will find this to be a positive experience! Contribution can
be
intimidating and we hope to alleviate that pain as much as possible.
Without
following these guidelines, you may be missing context that can help you
succeed
with your contribution, which is why we encourage discussion first.
Ultimately,
there is no guarantee that we will be able to merge your pull-request,
but by
following these guidelines we can try to avoid disappointment.

-->

Co-authored-by: bryn <bryn@apollographql.com>
The addition of the `-r` flag broke the existing logic for processing
the `-b` flag with non `-r` repos.

This fixes the logic and prints additional information about which repo
is being used to build the image.
Copy link
Contributor

@garypen garypen left a comment

Choose a reason for hiding this comment

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

The required helm chart updates haven't been done. (Chart.yaml: version and appVersion must be updated)

* `upgrade` - Upgrade the configuration with optional diff support.

`router --schema` has been deprecated and users should move to `router config schema`.

Copy link
Contributor

Choose a reason for hiding this comment

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

Missing PR details.

- name: user-agent
match: ^Mozilla/5.0 (iPhone*
headers: true
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing PR details.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```

By @bnjjj in #2040

- name: user-agent
match: ^Mozilla/5.0 (iPhone*
headers: true
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```

By @bnjjj in #2040


By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/2131

### Add configuration for logging and add more logs
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### Add configuration for logging and add more logs
### Add configuration for logging and add more logs ([Issue #1998](https://github.com/apollographql/router/issues/1998))

@BrynCooke
Copy link
Contributor Author

Closing as a real release PR will now be created

@BrynCooke BrynCooke closed this Dec 5, 2022
@BrynCooke BrynCooke deleted the 1.5.0 branch December 5, 2022 15:05
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.

8 participants