Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Telemetry] string_list metric type is being misused #12131

Closed
Dexterp37 opened this issue Jun 30, 2020 · 4 comments
Closed

[Telemetry] string_list metric type is being misused #12131

Dexterp37 opened this issue Jun 30, 2020 · 4 comments
Labels
🐞 bug Crashes, Something isn't working, .. Feature:Telemetry wontfix

Comments

@Dexterp37
Copy link
Contributor

Dexterp37 commented Jun 30, 2020

We just saw that #11118 and #11446: looks like string_list is being used to record single values that are bools.

This would make it hard for tools to understand the intent and pushes the burden of parsing data (which is now string data) on the analysis side, increasing both the cost and the likelihood of having bad data.

The code states the following:

        // We purposefully make all of our preferences the string_list format to make data analysis
        // simpler. While it makes things like booleans a bit more complicated, it means all our
        // preferences can be analyzed with the same dashboard and compared.

Setting aside the fact that that this will break GLAM, how does having a list of string having a single value of the wrong type make analysis simpler?

┆Issue is synchronized with this Jira Task

@Dexterp37 Dexterp37 added 🐞 bug Crashes, Something isn't working, .. Feature:Telemetry labels Jun 30, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Jun 30, 2020
@ekager
Copy link
Contributor

ekager commented Jun 30, 2020

cc @sblatz looks like this was a recent addition - do you know why we did this?

@Dexterp37
Copy link
Contributor Author

We had a conversation over Slack with @sblatz and Marissa Gorlick. Let me try to capture what was said, for future reference.

The decision to use string_list was made in order to make all "data types the same" to ease future dashboards, in response to Marissa's request for standardization.

While the intent was noble, we all agreed that this was not the right path to pursue, as "standardization" does not necessarily imply "all the is sent as string", which is both a problem for tools and for storage.

The path forward here is:

  • boolean values (toggle) can be a boolean type
  • metric that only record 1 string values, are string type
  • metrics that record more than 1 string value (e.g. accessibility services) are string_list type

While this is a very important change to make, right now we're in code freeze so it can't happen right away.

Any standardization can happen at analysis side, if needed, through UDFs.

I'm happy yo provide support with reviews as needed!

@Dexterp37
Copy link
Contributor Author

cc @mdboom

@data-sync-user data-sync-user changed the title [Telemetry] string_list metric type is being misused FNX3-15161 ⁃ [Telemetry] string_list metric type is being misused Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX3-15161 ⁃ [Telemetry] string_list metric type is being misused FNX-13160 ⁃ [Telemetry] string_list metric type is being misused Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX-13160 ⁃ [Telemetry] string_list metric type is being misused FNX2-12975 ⁃ [Telemetry] string_list metric type is being misused Aug 11, 2020
@kbrosnan kbrosnan changed the title FNX2-12975 ⁃ [Telemetry] string_list metric type is being misused [Telemetry] string_list metric type is being misused Aug 29, 2020
@ekager ekager removed the needs:triage Issue needs triage label Dec 17, 2020
@stale
Copy link

stale bot commented Jun 15, 2021

See: #17373 This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 15, 2021
@stale stale bot closed this as completed Jun 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. Feature:Telemetry wontfix
Projects
None yet
Development

No branches or pull requests

2 participants