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

Loglevel Trace causes Router to crash #3474

Closed
lleadbet opened this issue Jul 19, 2023 · 1 comment · Fixed by #3477
Closed

Loglevel Trace causes Router to crash #3474

lleadbet opened this issue Jul 19, 2023 · 1 comment · Fixed by #3477
Assignees

Comments

@lleadbet
Copy link
Contributor

Describe the bug

Reporting on behalf of a customer. They're reporting these issues (whenever loglevel is set to trace):

  • If you've enabled the TLS configuration with loglevel as trace, the Router will crash after some time, or potentially immediately
  • If you don't have any TLS configuration and view the sandbox and refresh, it crashes after a minute or so
  • if you've enabled the auth plugin and configured subgraph TLS along with --log=trace, the Router crashes on start

To Reproduce

Use this config:

authentication:
 jwt:
   jwks: # This key is required.
     - url: https://dev-zzp5enui.us.auth0.com/.well-known/jwks.json

   # These keys are optional. Default values are shown.
   header_name: Authorization
   header_value_prefix: Bearer

authorization:
  require_authentication: true

tls:
  subgraph:
    subgraphs:
      subgraph_name:
        certificate_authorities: "${file.config/cer.crt}"

Start the router with ./router --config router.yaml --log=trace and it should crash.

Expected behavior

Router to not crash.

Output

Last log before crashing over time (TLS only):

thread panicked while processing panic. aborting.
Abort trap: 6

Last logs before crashing on start (JWT + TLS)

{"timestamp":"2023-07-18T15:41:07.719052Z","level":"DEBUG","message":"loading root certificates","target":"apollo_router::router_factory"}
{"timestamp":"2023-07-18T15:41:07.719069Z","level":"TRACE","connection.state":"Open","target":"h2::proto::connection","span":{"field_error":"EOF while parsing a value at line 1 column 0","name":"poll"},"spans":[{"field_error":"EOF while parsing a value at line 1 column 0","name":"Connection"},{"field_error":"EOF while parsing a value at line 1 column 0","name":"poll"}]}
{"timestamp":"2023-07-18T15:41:07.719126Z","level":"TRACE","message":"poll","target":"h2::codec::framed_read","span":{"field_error":"EOF while parsing a value at line 1 column 0","name":"FramedRead::poll_next"},"spans":[{"field_error":"EOF while parsing a value at line 1 column 0","name":"Connection"},{"field_error":"EOF while parsing a value at line 1 column 0","name":"poll"},{"field_error":"EOF while parsing a value at line 1 column 0","name":"FramedRead::poll_next"}]}
{"timestamp":"2023-07-18T15:41:07.719530Z","level":"INFO","message":"Health check endpoint exposed at http://127.0.0.1:8088/health"}

Desktop (please complete the following information):

  • OS: macOS
  • Version v1.24.0

Additional context

The affected user also has been experiencing the issue noted in #3124, which may or may not be related. I (personally) have been unable to reproduce either issue, however.

@garypen garypen self-assigned this Jul 20, 2023
@garypen
Copy link
Contributor

garypen commented Jul 20, 2023

It looks like the error is in the h2 crate. You can use a log filter to set h2 loglevel to info as follows:

--log=trace,h2=info

This will give trace logging for all crates apart from h2 which will log at info level.

garypen added a commit that referenced this issue Jul 20, 2023
It would be nice if users could specify just a log level and the router
applied the required filtering to constrain the logging to the
`apollo_router` module.

It would also be nice if, for advanced users, you could exercise the
full power of a logging filter.

This PR enables both these use cases.

If you set a filter using `RUST_LOG`, it is used as is.

If you set it using `APOLLO_ROUTER_LOG` or `--log`, then any "global"
scope levels are constrained to `apollo_router`.

Thus:

```
RUST_LOG=apollo_router=warn
--log warn
APOLLO_ROUTER_LOG=warn
```

are equivalent with all three statements resulting in `warn` level
logging for the router.

For more details, read the logging configuration documentation.

fixes: #3474
garypen added a commit that referenced this issue Jul 21, 2023
)

It would be nice if users could specify just a log level and the router
applied the required filtering to constrain the logging to the
`apollo_router` module.

It would also be nice if, for advanced users, you could exercise the
full power of a logging filter.

This PR enables both these use cases.

If you set a filter using `RUST_LOG`, it is used as is.

If you set it using `APOLLO_ROUTER_LOG` or `--log`, then any "global"
scope levels are constrained to `apollo_router`.

Thus:

```
RUST_LOG=apollo_router=warn
--log warn
APOLLO_ROUTER_LOG=warn
```

are equivalent with all three statements resulting in `warn` level
logging for the router.

For more details, read the logging configuration documentation.

fixes: #3474

<!-- start metadata -->

**Checklist**

Complete the checklist (and note appropriate exceptions) before a final
PR is raised.

- [ ] Changes are compatible[^1]
- [x] Documentation[^2] completed
- [ ] Performance impact assessed and acceptable
- Tests added and passing[^3]
    - [x] 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.
- please raise a separate issue to automate the test and label it (or
ask for it to be labeled) as `manual test`

---------

Co-authored-by: Bryn Cooke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants