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

Remove experimental_when_header feature #6285

Open
wants to merge 6 commits into
base: next
Choose a base branch
from
Open

Remove experimental_when_header feature #6285

wants to merge 6 commits into from

Conversation

bnjjj
Copy link
Contributor

@bnjjj bnjjj commented Nov 19, 2024

If you used the experimental_when_header feature previously like this for example:

telemetry:
  exporters:
    logging:
      # If one of these headers matches we will log supergraph and subgraphs requests/responses
      experimental_when_header: # REMOVED
        - name: apollo-router-log-request
          value: my_client
          headers: true # default: false
          body: true # default: false

This feature no longer exists and can be replaced with another one included in custom telemetry. Here is how you would configure custom telemetry to achieve the same result:

telemetry:
  instrumentation:
    events:
      router:
        request: # Display router request log
          level: info
          condition:
            eq:
            - request_header: apollo-router-log-request
            - my_client
        response: # Display router response log
          level: info
          condition:
            eq:
            - request_header: apollo-router-log-request
            - my_client
      supergraph:
        request: # Display supergraph request log
          level: info
          condition:
            eq:
            - request_header: apollo-router-log-request
            - my_client
        response:
          level: info
          condition:
            eq:
            - request_header: apollo-router-log-request
            - my_client
      subgraph:
        request: # Display subgraph request log
          level: info
          condition:
            eq:
            - supergraph_request_header: apollo-router-log-request
            - my_client
        response: # Display subgraph response log
          level: info
          condition:
            eq:
            - supergraph_request_header: apollo-router-log-request
            - my_client

Signed-off-by: Benjamin <[email protected]>
@bnjjj bnjjj requested review from a team as code owners November 19, 2024 15:35
@svc-apollo-docs
Copy link
Collaborator

svc-apollo-docs commented Nov 19, 2024

✅ Docs Preview Ready

Configuration
{
  "repoOverrides": {
    "apollographql/router@next": {
      "remote": {
        "owner": "apollographql",
        "repo": "router",
        "branch": "bnjjj/fix_786"
      }
    }
  }
}
Name Link

Commit

fdbaa9b

Preview

https://www.apollographql.com/docs/deploy-preview/fddd42489b267968ec57

Build ID

fddd42489b267968ec57

File Changes

0 new, 1 changed, 0 removed
* graphos/reference/migration/from-router-v1.mdx

Pages

1


1 pages published. Build will be available for 30 days.

@router-perf
Copy link

router-perf bot commented Nov 19, 2024

CI performance tests

  • connectors-const - Connectors stress test that runs with a constant number of users
  • const - Basic stress test that runs with a constant number of users
  • demand-control-instrumented - A copy of the step test, but with demand control monitoring and metrics enabled
  • demand-control-uninstrumented - A copy of the step test, but with demand control monitoring enabled
  • enhanced-signature - Enhanced signature enabled
  • events - Stress test for events with a lot of users and deduplication ENABLED
  • events_big_cap_high_rate - Stress test for events with a lot of users, deduplication enabled and high rate event with a big queue capacity
  • events_big_cap_high_rate_callback - Stress test for events with a lot of users, deduplication enabled and high rate event with a big queue capacity using callback mode
  • events_callback - Stress test for events with a lot of users and deduplication ENABLED in callback mode
  • events_without_dedup - Stress test for events with a lot of users and deduplication DISABLED
  • events_without_dedup_callback - Stress test for events with a lot of users and deduplication DISABLED using callback mode
  • extended-reference-mode - Extended reference mode enabled
  • large-request - Stress test with a 1 MB request payload
  • no-tracing - Basic stress test, no tracing
  • reload - Reload test over a long period of time at a constant rate of users
  • step-jemalloc-tuning - Clone of the basic stress test for jemalloc tuning
  • step-local-metrics - Field stats that are generated from the router rather than FTV1
  • step-with-prometheus - A copy of the step test with the Prometheus metrics exporter enabled
  • step - Basic stress test that steps up the number of users over time
  • xlarge-request - Stress test with 10 MB request payload
  • xxlarge-request - Stress test with 100 MB request payload

@abernix
Copy link
Member

abernix commented Nov 19, 2024

@bnjjj @garypen Does this get a place in some section of the migration guide?

@bnjjj
Copy link
Contributor Author

bnjjj commented Nov 20, 2024

@abernix Yes. Do we have an existing one somewhere ?

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.

Needs an entry in the migration guide.

apollo-router/src/plugins/telemetry/config_new/logging.rs Outdated Show resolved Hide resolved
apollo-router/tests/integration/lifecycle.rs Outdated Show resolved Hide resolved
@@ -89,6 +89,7 @@ The subgraph service executes multiple times during query execution, with each e
| `supergraph_operation_kind` | Yes | `string` | The operation kind from the supergraph query |
| `supergraph_query` | Yes | `string` | The graphql query to the supergraph |
| `supergraph_query_variable` | Yes | | The name of a supergraph query variable |
| `supergraph_request_header` | Yes | | The name of a supergraph request header |
Copy link
Member

Choose a reason for hiding this comment

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

is this something that should be added on dev/main rather than next?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add it on dev too

@bnjjj bnjjj requested a review from garypen November 21, 2024 15:14
@bnjjj bnjjj enabled auto-merge (squash) November 22, 2024 14:31
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.

5 participants