Skip to content

feat: add instrumentaion to first mile journey#9193

Merged
AdityaHegde merged 8 commits intomainfrom
feat/instrument-first-mile-journey
Apr 13, 2026
Merged

feat: add instrumentaion to first mile journey#9193
AdityaHegde merged 8 commits intomainfrom
feat/instrument-first-mile-journey

Conversation

@AdityaHegde
Copy link
Copy Markdown
Collaborator

We need to know when users fall off. So adding instrumentation to identify step transitions, including errors and going back in the flow.

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@AdityaHegde AdityaHegde requested a review from ericokuma April 7, 2026 06:06
@AdityaHegde AdityaHegde requested a review from ericpgreen2 April 8, 2026 03:49
@AdityaHegde AdityaHegde force-pushed the feat/instrument-first-mile-journey branch from 8afef7c to 61c55d0 Compare April 8, 2026 03:54
@AdityaHegde AdityaHegde force-pushed the feat/instrument-first-mile-journey branch from 792020e to 1c4a5cf Compare April 9, 2026 04:58
Copy link
Copy Markdown
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

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

1. Consider centralizing telemetry in pushState/popState rather than sprinkling it through transition()

The state manager already knows everything needed to derive the telemetry payload: the current state (schema, connector, step) and the direction (forward vs. back). Instead of instrumenting each case branch individually, the telemetry could fire automatically from pushState and popState, with a mapping from state → action + fields. This would eliminate the scattered inline calls, make it impossible to miss a step, and keep the telemetry concern in two focused methods.

There's an existing TODO at BehaviourEventHandler.ts:19 acknowledging the broader telemetry layer verbosity. This PR is a good example of why that simplification is worth doing — the handler → factory → service chain requires a new method at each layer for what is conceptually "fire event with action name and some fields."

2. Telemetry gap: no event fires for the SelectConnector step

The old code in AddAssetButton.svelte fired a SourceAdd event when the user opened the add-data modal. That call is now removed. The new instrumentation fires step events inside the state machine, but the post-transition switch (AddDataStateManager.svelte.ts:207-249) only covers CreateConnector, CreateModel, ExploreConnector, and Import. The SelectConnector step — the first screen most users see — fires no event.

This means there's no way to tell how many users enter the flow and immediately close it, which seems like an important drop-off point given the stated goal of "know when users fall off."

3. fireErrorEvent — the step parameter vs. this.state.step for field population

fireErrorEvent accepts a step parameter (used in the event payload) but always reads this.state.step to decide which fields (schema, connector) to populate. In GenerateDashboardStatus.svelte:100, it's called with importStep (an ImportDataStep like "create-model"), while this.state.step is AddDataStep.Import. This is correct — the sub-step goes in the event, and the broader state gives context — but the dual meaning of "step" is confusing. Consider renaming the parameter to subStep or adding a brief comment explaining the distinction.

4. Minor: AddDataConfig mixes UI and telemetry concerns

AddDataConfig now holds both UI flags (welcomeScreen, importOnly) and telemetry fields (medium, space, screen). This works since all telemetry fields are optional, but if telemetry config grows, the type becomes muddled. A nested telemetry?: { medium, space, screen } would keep concerns separate.


Developed in collaboration with Claude Code

@AdityaHegde AdityaHegde merged commit c703a12 into main Apr 13, 2026
10 checks passed
@AdityaHegde AdityaHegde deleted the feat/instrument-first-mile-journey branch April 13, 2026 14:09
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.

3 participants