Add WebSocket vital event schemas - #429
Draft
bdibon wants to merge 2 commits into
Draft
Conversation
Report WebSocket connection metrics as a stream of vital events, one per connection phase, all sharing a `vital.websocket.id`. Adds one schema per phase, discriminated by a const on `vital.name` over the shared `vital.type` const `websocket`, following the pattern the timeseries event schemas already use. Registered in `rum-events-schema.json` and `rum-events-browser-schema.json` only: the browser SDK is the only SDK reporting these for now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`dd-sdk-android`'s model generator cannot narrow a property that a shared schema already declares: `Primitive.mergedWith` accepts only an identical `Primitive`, while `Constant.mergedWith` and `Enum.mergedWith` error unconditionally (`buildSrc/.../jsonschema/TypeDefinition.kt`). Since `_vital-common-schema.json` declares `vital.name` as a free-form string, a `const` on it in each phase schema fails `generateRumModelsFromJson` with "Can't merge Primitive with type Constant". Drop the consts. The four phase schemas stay mutually exclusive through their disjoint `required` sets, so `oneOf` still resolves to exactly one arm per event. `vital.name` remains required, declared as the plain string the shared envelope already uses, and each phase schema records the value it is reported with in its description. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this adds
A
websocketvital type, reporting a WebSocket connection as a stream of vitals over its lifetime — one per connection phase, all sharing avital.websocket.idthat the backend reducer groups on.vital.websocket.idis the connection id and is deliberately notvital.id, which stays a fresh UUID per event.Four phase schemas over a shared common schema, plus two shared fragments:
_vital-websocket-common-schema.jsonvital.typeconstwebsocket, requiredvital.name, andvital.websocket.id— the only websocket field required on all four phasesvital-websocket-connecting-schema.jsonurl,connecting_datevital-websocket-open-schema.jsonsnapshot_version,snapshotvital-websocket-closing-schema.jsonclosing_date,close_initiatorvital-websocket-closed-schema.jsonclosed_date,duration,tracking_end_reason,snapshot_version_vital-websocket-snapshot-schema.jsonsnapshot.inbound/snapshot.outbound, shared by the open and closed phases_vital-websocket-message-direction-schema.json$ref'd by both directionsRegistered in the
RumVitalEventoneOfofrum-events-schema.jsonandrum-events-browser-schema.json. Not registered inrum-events-mobile-schema.json— the browser SDK is the only SDK reporting these today. That does not keep them away from mobile, though: per the README the Android SDK generates models from everything underschemas/rum/, which the CI run on this PR confirmed.How the four phases are discriminated
By their disjoint
requiredsets, not byvital.name. The natural design would put aconstonvital.namein each phase schema. That cannot work:dd-sdk-android's generator cannot narrow a property a shared schema already declares —Primitive.mergedWithaccepts only an identicalPrimitive, and bothConstant.mergedWithandEnum.mergedWitherror unconditionally (TypeDefinition.kt). Since_vital-common-schema.jsondeclaresvital.nameas a free-form string, aconstor anenumon it failsgenerateRumModelsFromJsonwithCan't merge Primitive with type Constant.So
vital.nameis required but unconstrained, and each phase schema records the value it is reported with in itsdescription. Consequences reviewers should weigh:vital.namedisagrees with its field set validates — the name is not checked against the phase.oneOfdemands exactly one match.RumEventcannot narrow to a phase byvital.name. Per-phase builders that annotate their return type are unaffected.If the Android limitation is lifted, adding the four consts back is a small change here.
Judgement calls worth pushing back on
1. Four per-phase schemas rather than one all-optional
vital.websocketobject. The alternative was a single schema with every field butidoptional and the per-phase presence rules left to descriptions. Per-phase files make each phase'srequiredset machine-checked instead of prose.RumVitalEventgoes from 3 arms to 7 as a result.2. Per-phase
requiredsets, against the README's "new fields must not be set as required". That rule protects existing event shapes: no historical event hasvital.type: "websocket", so nothing old can start failing. The forward cost is that these sets now constrain the SDK — relaxing one later is a breaking change.timeseries-memory-schema.jsonandtimeseries-cpu-schema.jsonrequire six fields each on the same reasoning.3.
vital.websocket.idcarries the same UUIDpatternasvital.id. Tightens the reducer's grouping key to a UUID.4. The
close_event-only rule forclose_code/close_reason/was_cleanis description-only. Anif/thenexpressing it is rejected by ajv'sstrictRequiredinyarn validateunless the three properties are re-declared insidethen; the duplication wasn't worth it.5. The per-phase split stops at the snapshot.
silence_before_closeandbuffered_amount_at_closeare CLOSED-only but live in the shared snapshot fragments as optional, so they would also validate on an OPEN heartbeat. Splitting the snapshot per phase costs two more files and produces messier generated types.6. Nothing here rejects an unknown field. The
rum/schemas don't setadditionalProperties: false, and it isn't usable with this composition anyway — inallOfeach arm validates the whole object, so an arm forbidding extras would rejectvital.websocket.idfrom the sibling common arm. Consequence: fields deliberately not part of this model still validate if emitted —closing_durationon CLOSED (the reducer computes it asclosed_date − closing_date),close_initiatoron CLOSED (the reducer derives it), andready_state. They are absent from the schema rather than rejected by it.Test instructions
Six samples cover all four phases, plus a CLOSED with
tracking_end_reason: session_end(noCloseEventfields) and a CLOSED for a connection that never opened (nosnapshot). The pre-existingsamples/rum-events/vital.jsonstill validates, which is what confirms theoneOfresolves to exactly one arm per event.Separately verified: the four sample payloads published in the WebSocket Data Model RFC validate against these schemas unchanged, and each of the six new samples matches exactly one phase arm while the existing duration vital matches none.