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

Logging configuration to enable/disable specific logs #1998

Closed
bnjjj opened this issue Oct 25, 2022 · 4 comments · Fixed by #2040
Closed

Logging configuration to enable/disable specific logs #1998

bnjjj opened this issue Oct 25, 2022 · 4 comments · Fixed by #2040
Assignees

Comments

@bnjjj
Copy link
Contributor

bnjjj commented Oct 25, 2022

Subtask of #1840

Here are different versions I have in mind to enable/disable different logs:

telemetry:
  logging:
    format: pretty # Could be json or pretty
    router:
      request:
        enabled: true
        level: trace # Display with trace level (default: debug)
      response:
        on_graphql_error: true # Display a log when we return a GraphQL error
        on_http_error: true # Display a log if status code >200
        always: false # If enabled we log every requests
        level: info # Display with info level (default: debug)

    subgraphs:
      all:
        request: 
          enabled: true
          level: debug # Display with debug level (default: debug)
        response: 
          on_graphql_error: true # Display a log when we return a GraphQL error
          on_http_error: true # Display a log if status code >200
          always: false # If enabled we log every requests
      my_subgraph:
        request: 
          enabled: true
        response:
          on_graphql_error: true # Display a log when we return a GraphQL error
          on_http_error: true # Display a log if status code >200
          level: info # Display with info level (default: debug)
          always: false # If enabled we log every requests

or with a mode:

telemetry:
  logging:
    format: pretty # Could be json or pretty
    router:
      request:
        enabled: true
        level: info # log the request on info level
      response:
        # on_error includes http_error and graphql_error 
        mode: on_error # on_error | always
        level: debug # log the response on debug level
    subgraphs:
      all:
        request:
          enabled: true
          level: trace # log the response on trace level
        response: 
          # on_error includes http_error and graphql_error 
          mode: on_error # on_error | always
      my_subgraph:
        request: 
          enabled: true
        response:
          # on_error includes http_error and graphql_error 
          mode: on_error # on_error | always
@bnjjj bnjjj self-assigned this Oct 25, 2022
@bnjjj bnjjj mentioned this issue Oct 25, 2022
4 tasks
@bnjjj
Copy link
Contributor Author

bnjjj commented Oct 26, 2022

Maybe let's start with something smaller, less features (like level and redact). And add them in the future if needed.

telemetry:
  logging:
    format: pretty # Could be json or pretty
    router:
      request:
        enabled: true
      response:
        # on_error includes http_error and graphql_error 
        mode: on_error # on_error | always
    subgraphs:
      all:
        request:
          enabled: true
        response: 
          # on_error includes http_error and graphql_error 
          mode: on_error # on_error | always
      my_subgraph:
        request: 
          enabled: true
        response:
          # on_error includes http_error and graphql_error 
          mode: on_error # on_error | always

@bnjjj
Copy link
Contributor Author

bnjjj commented Nov 17, 2022

Previous proposal with attributes (to keep history):

telemetry:
  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
    supergraph:
      contains_attributes: # Available attributes are request|response_body|response_headers|operation_name
        - request # Display the request at the supergraph level
    subgraph:
      all: # Display logs that contains these attributes
        contains_attributes: # Available attributes are request|response_body|response_headers|operation_name
          - response_body # Display the response_body for all the subgraph calls
      subgraphs:
        accounts: # Display logs that contains these attributes
          contains_attributes: # Available attributes are request|response_body|response_headers|operation_name
            - response_headers # Display the response_headers for the subgraph "accounts"

@bnjjj
Copy link
Contributor Author

bnjjj commented Nov 17, 2022

Ok after feedback and some requirements I reshaped a new configuration:

telemetry:
  experimental_logging:
    request:
      # If one of these headers matches
      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
    response:
      when_header:
        - name: apollo-router-log-response
          value: my_client
          body: true

Here are different scenarios covered:

  • As an admin of the router a client has a bug/error related to the router and I want to debug response received by subgraphs. --> Add new header in the response list or use an existing one and give it to the client to add in the request.
  • If we can't put a specific header on the client side but still wants to debug, let's add a catch all header like name: method with match: * and it will log all responses from subgraphs and for the supergraph.
  • Same if we just want to enable those logs for everyone
  • If I no longer want to log something on this header of with a specific value, just delete the header entry.

Pros:

  • It's really dynamic
  • Doesn't require the client to be updated if we can't
  • We can revoke this ability for a specific client
  • If a bug only appears on a specific platform we can still play with user-agent header

Cons:

  • We can't choose to enable only logs for specific subgraphs.
  • When we enable response body it will log the response body for each subgraphs AND for the supergraph
  • TLDR it's a little bit less fine-graded configuration

@bnjjj
Copy link
Contributor Author

bnjjj commented Nov 18, 2022

Decision was to remove request and response:

telemetry:
  experimental_logging:
    # 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

We do not need to specify request or response and we still can play with RUST_LOG env variable to filter specific logs

bnjjj added a commit that referenced this issue Nov 24, 2022
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>
garypen pushed a commit that referenced this issue Nov 30, 2022
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>
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.

1 participant