Enable subgraph metrics and extended error metrics by default - #9879
Enable subgraph metrics and extended error metrics by default#9879BobaFetters wants to merge 10 commits into
Conversation
Router 3.0 changes the defaults for two Apollo telemetry settings to match the preferred configuration for GraphOS Studio users, and promotes one of them out of preview.
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 1 new, 20 changed, 0 removedBuild ID: ac40c0cde81394b880c2585b URL: https://www.apollographql.com/docs/deploy-preview/ac40c0cde81394b880c2585b
|
|
/claude-review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| router.start().await; | ||
| router | ||
| .wait_for_log_message(r#""subgraph":"connectors","message":"plugin `telemetry` is indirectly configured to send errors to Apollo studio for a connector-enabled subgraph, which is only supported when `preview_extended_error_metrics` is enabled"#) | ||
| .wait_for_log_message(r#""subgraph":"connectors","message":"plugin `telemetry` is indirectly configured to send errors to Apollo studio for a connector-enabled subgraph, which is only supported when `extended_error_metrics` is enabled"#) |
There was a problem hiding this comment.
Bug: this test (and incompatible_warnings_on_subgraph below) is broken by the new extended_error_metrics default.
Both incompatibility warnings are gated on the mode not being Enabled (incompatible/telemetry.rs#L22-L29):
self.config.subgraph.all.send
&& !matches!(
self.config.extended_error_metrics,
apollo::ExtendedErrorMetricsMode::Enabled
)Neither the config of this test (L747-L756) nor that of incompatible_warnings_on_subgraph (L784-L796) sets extended_error_metrics, so with the new default of enabled from this PR the warning is never emitted and wait_for_log_message panics after its 30s deadline. The expected log strings were updated for the rename, but the configs were not updated to preserve the pre-condition the tests rely on.
This is easy to miss because both tests early-return when graph_os_enabled() is false (no TEST_APOLLO_KEY/TEST_APOLLO_GRAPH_REF), so the failure only shows up on credentialed runs.
Fix: add extended_error_metrics: disabled under errors: in both test configs, e.g.:
telemetry:
apollo:
errors:
extended_error_metrics: disabled
subgraph:
all:
send: true
Code reviewReviewed the current diff (head
Otherwise the change checks out: the rename is complete across code/tests/docs, the |
# Conflicts: # docs/source/routing/configuration/yaml.mdx
…y tests extended_error_metrics now defaults to Enabled, which suppresses the incompatibility warnings these tests assert on, causing wait_for_log_message to hang until timeout on credentialed runs.
|
/claude-review |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
…metrics defaults Both now default to enabled, so router.operations.fetch.duration and the extended error span events/status show up in previously-captured snapshots that never opted out. Purely additive; no behavior change.
goto-bus-stop
left a comment
goto-bus-stop
left a comment
There was a problem hiding this comment.
Code change LGTM. We should improve the docs though or file a follow-up ticket for that.
Address PR review feedback: the "enabling" framing read oddly now that extended_error_metrics defaults to enabled. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 participants
Router 3.0 changes the defaults for two Apollo telemetry settings to match the preferred configuration for GraphOS Studio users, and promotes one of them out of preview.
telemetry.apollo.subgraph_metricsnow defaults totrue. Subgraph metrics send additional per-subgraph operation metrics to GraphOS Studio via OTLP, powering subgraph insights. Previously this was opt-in. To restore the previous behavior, set:telemetry.apollo.errors.preview_extended_error_metricshas been renamed totelemetry.apollo.errors.extended_error_metricsand now defaults toenabled. Extended error metrics send OTLP error metrics with additional dimensions (extensions.service,extensions.code), giving Studio richer error attribution out of the box. Thepreview_prefix has been dropped now that the feature is stable.Configurations using the old
preview_extended_error_metricsfield name are migrated automatically at startup (with a warning). To restore the previous behavior, set: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. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
A lot of (if not most) features benefit from built-in observability and
debug-level logs. Please read this guidance on metrics best-practices. ↩Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩