@diosmosis opened this Pull Request on November 23rd 2020 Member

Description:

Introduces a new core metric, nb_profilable calculated with VisitsSummary metrics as the sum of visits that are profilable. If less than 1% of visits for a period are not profilable features that depend on profilable data are hidden or disabled. This includes:

  • reports that rely on detecting returning visitors (everywhere in the UI)
  • the nb_uniq_visitors metric (in the UI and API)
  • segments for dimensions that rely on detecting returning visitors (hidden in the UI only)
  • visitor log features that rely on detecting returning visitors

Other changes:

  • nb_profilable is stored even if it's value is 0, this is to be able to differentiate between archives that are old and do not have the metric vs ones that are new and have 0 profilable visits. There is an alternative to check whether ts_archived is < a certain time, but getting this information w/ numeric data is a bit complicated. It might be do-able though.

Fixes #16363

Review

  • [ ] Functional review done
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@diosmosis commented on December 21st 2020 Member

@tsteur / @sgiehl this is ready for an initial review

@diosmosis commented on January 14th 2021 Member

@tsteur @sgiehl this review is ready for review. could use a general review of the strategy used first.

@diosmosis commented on January 17th 2021 Member

wondering generally instead of removing some reports to maybe rather show a notice like "This report won't show any data because no profilable visitors" (would need to word it better).

:+1: this makes sense to me. It would be strange to just click a report and suddenly not be able to see a report. I'll make this change?

@tsteur commented on January 17th 2021 Member

I'll make this change?
sounds great

@diosmosis commented on January 18th 2021 Member

@mattab can you provide a UI review + look at the comments on the review and provide your thoughts?

Powered by GitHub Issue Mirror