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

feat(experiments): improve "no results" diagnostics #27772

Merged
merged 3 commits into from
Jan 27, 2025

Conversation

jurajmajerik
Copy link
Contributor

@jurajmajerik jurajmajerik commented Jan 22, 2025

Problem

When solving this ticket, I've noticed that the "no results" diagnostics weren't working entirely correctly. This user did have the required event, just was missing the feature flag information on them. Yet, the diagnostics popup showed "Metric events not received".

I'm a bit unsure why this used to work in the old code. We did have this filter condition, but the query must've returned counts for all events too (to be able to confirm their existence in the validation method). In the new HogQL implementation, this is not the case, and only the events having the feature flag variants get returned.

Changes

Rather than fixing this, I'm proposing the following change:

  • Simplify the diagnostics list, where we only let the user know when there are no events with required variants, as opposed to no events whatsoever.
  • In the popup, add links to the Activity tab where the user can:
    • Verify that there are indeed no said events
    • Play with the filter to see if they're able to find any events at all

This is simplifies the checklist and lets the user verify and debug missing events in a more interactive way.

Trends

Before After
image image

Funnels

Before After
image image
  • Clicking the link takes you to the activity tab where you can verify missing events:
image

How did you test this code?

Manually tested that the Activity query is set up correctly for:

  • Trends, default exposure
  • Trends custom exposure
  • Funnels

@jurajmajerik jurajmajerik requested a review from a team January 22, 2025 11:41
Copy link
Contributor

github-actions bot commented Jan 22, 2025

Size Change: 0 B

Total Size: 1.16 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.16 MB

compressed-size-action

Copy link
Contributor

@andehen andehen left a comment

Choose a reason for hiding this comment

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

Great to have this checklist improved!

Tested an experiment with only control events and it looks like this then
Screenshot 2025-01-22 at 13 29 04

Here is the error message from the query API in that example:

`[ErrorDetail(string='{"no-exposures": false, "no-control-variant": false, "no-test-variant": true, "no-flag-info": false}', code='invalid')]`

Also, I find the "double negation" in the messages a little confusing actually. I think it would be clearer to have the success text with a red cross, so without the not. Not a strong opinion, but my personal preference.

@jurajmajerik
Copy link
Contributor Author

@andehen ah it's a cached response! Good catch, fixed here: d0868c9 (#27772)

I think it would be clearer to have the success text with a red cross, so without the not.

We had this discussion a few months ago. For some reason, either option seems slightly confusing to some people. This is what we went with at the end 😶

@jurajmajerik jurajmajerik requested a review from andehen January 22, 2025 14:16
@jurajmajerik jurajmajerik merged commit 8e5183a into master Jan 27, 2025
99 checks passed
@jurajmajerik jurajmajerik deleted the experiment-better-diagnostics branch January 27, 2025 12:58
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.

2 participants