Skip to content

Declare telemetry's instrumentation config with the configuration attribute - #10328

Draft
BrynCooke wants to merge 3 commits into
dev-v3.xfrom
lane/ROUTER-2143
Draft

BrynCooke wants to merge 3 commits into
dev-v3.xfrom
lane/ROUTER-2143

Conversation

@BrynCooke

@BrynCooke BrynCooke commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Telemetry's instrumentation configuration (events, spans and instruments, with their attributes, selectors and conditions) is now declared with apollo-configuration's #[configuration] attribute or has an explicit Validate impl. The telemetry plugin's last skip_validate field is gone, so validation reaches every part of the telemetry configuration while the file is parsed.

This is the third and final PR for ROUTER-2143, which converts telemetry's exporters, apollo and instrumentation subtrees, one PR per subtree. #10326 (telemetry exporters) and #10327 (telemetry Apollo reporting) have merged, so this PR is now based directly on dev-v3.x. Both followed #10317 for ROUTER-2142, part of ROUTER-2132. #10326 explains the conversion pattern and the parse-time validation path.

Intent

Two sections used to ignore keys they didn't define, and now reject them:

  • Custom events. In telemetry.instrumentation.events.router.my.event, a misspelled levle: warn beside level: info was dropped.
  • The level and condition form of the standard request, response and error events. A conditon: typo next to a real condition: was dropped.

Both are in the breaking changeset and the v3 upgrade guide's unknown-key note. Everything else in the section parses as before; in the schema snapshot, the event definitions gain additionalProperties: false and optional selector fields gain default: null. The plugin's existing selector-stage checks still only log warnings; this PR doesn't change them.

Why this approach

Most types take the macro directly. Fields that serde treated as required keep #[config(required)]. That matters most for the untagged selector enums: each variant is told apart by its key (for example connector_http_method), and if that key defaulted, an empty map would match the first variant. The schema snapshot's required lists are unchanged, which confirms none was lost.

Three groups of types can't use the macro. Each has a small hand-written Validate impl with a comment:

  • Generic types. apollo-configuration's macro emits its Default and Validate impls without the type's generic parameters, so it can't compile a generic type. Extendable, Conditional, Condition, SelectorOrValue, StandardEventConfig, the custom Event, DefaultedStandardInstrument, Instrument, InstrumentValue and the instrument Event and Field keep their serde and schemars derives. Their impls forward validation to their nested sections under the same keys the macro would use; untagged enums validate at their own path. These impls are temporary debt: a follow-up apollo-configuration ticket adds generic support to the macro, after which they can be removed and the types converted. This PR doesn't change apollo-configuration.
  • Router, supergraph and subgraph selectors. Their response-error selectors hold a JsonPathInst, which has no Debug impl for the macro's derive, nor a Validate impl. Every other field is a scalar or unit enum, so their Validate impls are empty. This is the same reason ROUTER-2142 kept the headers plugin's Operation hand-written.
  • AttributeValue. This PR adds its impl. One variant wraps a plain f64, which has no Validate impl, so the macro can't derive one for that variant. No variant has a rule to check, so the impl is empty. Every selector's static and default fields, and SelectorOrValue's constant value, use it.

The first commit is a mechanical rename. Seven types had an inherent validate method that checks selector stages. Once they also implement the Validate trait, the macro's generated self.field.validate(errors) call resolves to the inherent method first and doesn't compile. Renaming the inherent methods to validate_selectors keeps the two apart; behaviour is unchanged.

services::header_masking::RedactMode, a unit enum the selectors use, converts too. The connector and HTTP client selectors drop derivative for a plain PartialEq derive, because the macro now provides Debug.

How it was verified

  • The unknown-key table test gains a case for a custom event and for a conditional standard event. A temporary positive control confirmed the same YAML without the typo parses.
  • The project config test (every router.yaml, docs example and dockerfile config) passes with the stricter types. Migration 2048-static-string-selector-deprecated rewrites values under event attributes, not keys at the event level, and passes.

Risk

Breaking, v3 only: an unknown key in a custom event, or in the level/condition form of a standard event, now fails to load. router config validate reports it.

@apollo-librarian

apollo-librarian Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

✅ Docs preview ready

The preview is ready to be viewed. View the preview

File Changes

0 new, 1 changed, 0 removed
* graphos/routing/v3/upgrade/from-router-v2.mdx

Build ID: f9aba8c187564ae9716c6384
Build Logs: View logs

URL: https://www.apollographql.com/docs/deploy-preview/f9aba8c187564ae9716c6384


⚠️ AI Style Review — 1 Issue Found

Summary

The pull request updates the documentation to align with style guide standards across several areas: Framing Apollo Products (shifting focus to the benefits of accurate configuration), Language (correcting 'request' to American English), Structural Elements (refactoring long list items for better scannability), Text Formatting (applying code font to plugin names and configuration keys), Verb Tense and Voice (preferring active voice over passive), and Word/Symbol Usage (standardizing on the Oxford comma).

Duration: 3017ms
Review Log: View detailed log

This review is AI-generated. Please use common sense when accepting these suggestions, as they may not always be accurate or appropriate for your specific context.

@BrynCooke
BrynCooke added this pull request to stack #10310 September 29, 2026 22:13
@BrynCooke
BrynCooke force-pushed the lane/ROUTER-2143 branch 2 times, most recently from 71fd6b0 to bbafdac Compare October 5, 2026 10:55
@BrynCooke
BrynCooke force-pushed the lane/ROUTER-2143 branch 2 times, most recently from 0c12fec to 704b543 Compare October 8, 2026 08:49
@BrynCooke
BrynCooke force-pushed the lane/ROUTER-2143 branch 2 times, most recently from f6c1082 to c8855c0 Compare October 13, 2026 18:03
Base automatically changed from lane/ROUTER-2143-split-2 to dev-v3.x October 14, 2026 10:01
The instrumentation, events, spans, instruments, condition, conditional
and extendable types each have an inherent validate method. It checks
that selectors run at a stage where they can produce a value, and the
telemetry plugin logs any problem as a warning.

Once these types implement apollo-configuration's Validate trait, the
configuration attribute's generated code calls validate with an error
collector, and method resolution would pick the inherent method instead.
Renaming the inherent methods to validate_selectors keeps the two apart.
Behavior is unchanged.

Part of ROUTER-2143.
The events, spans and instruments sections and their attribute, selector
and value types now use the configuration attribute, so the telemetry
instrumentation field no longer needs skip_validate. The header masking
RedactMode enum that several selectors use converts too. Fields that
serde treated as required, such as each selector's key in its untagged
enum, stay required so a selector still has to name what it selects.

Three groups of types keep hand-written Validate impls:
- The attribute doesn't support generic types. Extendable, Conditional,
  Condition, SelectorOrValue, StandardEventConfig, the custom event, and
  the instrument and instrument value types keep their serde and
  schemars derives and forward validation to their nested sections.
- The router, supergraph and subgraph selectors hold a JsonPathInst,
  which has no Debug impl for the macro's derive, nor a Validate impl.
  Their other fields are scalars without rules, so their impls are empty.
- AttributeValue's F64 variant wraps a plain f64, which has no Validate
  impl, so the attribute can't derive one for that variant. No variant
  has a rule to check, so its impl is empty. Every selector's `static`
  and `default` fields and SelectorOrValue's constant value use it.

Custom events and the conditional form of the standard request,
response and error events accepted unknown keys. Both now reject them.
The schema otherwise gains only null default annotations.

Part of ROUTER-2143.
Custom events and the conditional form of the standard request, response
and error events now reject unknown keys. No project config file, doc
example or migration puts extra keys there, and the configuration usage
metrics read only known keys from them.

The unknown-key table test gains a case for each, and the changeset and
the v3 upgrade guide name them next to the batch processor blocks.

Part of ROUTER-2143.
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.

1 participant