Adds missing schemas + data for metrics to snowplow stats ping #48476

escherize · 2024-10-08T17:33:09Z

resolves #48424

Most of the info for this section was in the legacy stats info. The new stuff is calculated in ->snowplow-metric-info.

The specified tags are included with "metrics".

Also, I refactored this to be more repl-friendly. We can call generate-instance-stats! from the repl to see its value.

This adds the jsonschema for "metrics" section, as well as "grouped_metrics".

- and add defaults

- we aren't reaching into the api API, so the linter is more of a formality here.

Somtom

Really excited to see our progress on this migration. I left a few questions and one blocking nitpick that should be easy to resolve :)

snowplow/iglu-client-embedded/schemas/com.metabase/instance_stats/jsonschema/1-0-0

- revert changes to 1-0-0 - add metrics section to new file: 1-0-1 - bump ::instance_stats to "1-0-1" - add tags into 1-0-1 - make the code return tags (empty for now until we know what to tag things.) - also fix test that broke from shuffling settings around

…tabase into add-metrics-to-snowplow-ping

snowplow/iglu-client-embedded/schemas/com.metabase/instance_stats/jsonschema/1-0-1

add grouped_metrics to jsonschema 1-0-1 format 1-0-1

snowplow/iglu-client-embedded/schemas/com.metabase/instance_stats/jsonschema/1-0-1

…tabase into add-metrics-to-snowplow-ping

- alphabetize metrics generation

escherize · 2024-10-11T22:36:21Z

I've tested this with snowplow mini, and it accepts the value 🎉.

Open question:

According to the snowplow docs, adding a required field (as we are in this PR) is a breaking change and should mean bumping to 2-0-0. However I also read that we generally shouldn't need to do that, so I could use some clarity there.

Answered:

Let's go with a 2-0-0 version.

- add maxLength to some strings

- Add description to setting.items.tags - add maxLength, and {min,max}imum to setting.items.value

Somtom

Sorry for the back and forth but there were new infos based on the discussion with SnowcatCloud

snowplow/iglu-client-embedded/schemas/com.metabase/instance_stats/jsonschema/1-0-1

src/metabase/analytics/snowplow.clj

snowplow/iglu-client-embedded/schemas/com.metabase/instance_stats/jsonschema/1-0-1

src/metabase/analytics/stats.clj

albertoperdomo · 2024-10-15T09:47:16Z

Should this not be backported to the 51 release branch so it makes it into 51? I see the no-backport label.

escherize · 2024-10-15T15:32:56Z

Should this not be backported to the 51 release branch so it makes it into 51? I see the no-backport label.

Thanks, I've switched the labels so we will backport it.

- remove analytics_uuid string length == 36 check - adds assertion to ensure required fields are set (and it passes) - adds info for embedding settings

- adds length info to the schema to pass jsonschema linting

escherize · 2024-10-15T17:09:47Z

Since we don't have telemetry data for embedding usage, we've expedited that piece and added the query_executions_by_source grouped metric in this PR.

* adds data + schema for metrics stats ping * remove comment * annotate todos * fill in the rest of the metrics values - and add defaults * fix some definitions + use a single timestamp * shuffle stuff around to appease the namespace linter - we aren't reaching into the api API, so the linter is more of a formality here. * update docstring * fix jsonschema, maybe * review responses - revert changes to 1-0-0 - add metrics section to new file: 1-0-1 - bump ::instance_stats to "1-0-1" - add tags into 1-0-1 - make the code return tags (empty for now until we know what to tag things.) - also fix test that broke from shuffling settings around * remove a footgun * add tags to metrics, add grouped_metrics to jsonschema 1-0-1 format 1-0-1 * indent * cljfmt * version should match filename * update instance stats 1-0-1 schema * require `tags` in grouped_metric + snowcat comment * fix formatting noise * update schema to make it valid * remove grouped_metrics from instance_stats schema * cljfmt appeasement * unbin cache_num_queries_cached value - alphabetize metrics generation * we can now guarantee metric values will be ints * jsonschema for instance_uuid, settings, and grouped_metrics * add analytics_uuid and make it required * lint + alphabetize instance stats json schema * update setting key type - add maxLength to some strings * lint jsonschema - Add description to setting.items.tags - add maxLength, and {min,max}imum to setting.items.value * Bump instance stats to 2-0-0 - remove analytics_uuid string length == 36 check - adds assertion to ensure required fields are set (and it passes) - adds info for embedding settings * adds a grouped-metric to stats ping - adds length info to the schema to pass jsonschema linting * cljfmt

… (#48760) * adds data + schema for metrics stats ping * remove comment * annotate todos * fill in the rest of the metrics values - and add defaults * fix some definitions + use a single timestamp * shuffle stuff around to appease the namespace linter - we aren't reaching into the api API, so the linter is more of a formality here. * update docstring * fix jsonschema, maybe * review responses - revert changes to 1-0-0 - add metrics section to new file: 1-0-1 - bump ::instance_stats to "1-0-1" - add tags into 1-0-1 - make the code return tags (empty for now until we know what to tag things.) - also fix test that broke from shuffling settings around * remove a footgun * add tags to metrics, add grouped_metrics to jsonschema 1-0-1 format 1-0-1 * indent * cljfmt * version should match filename * update instance stats 1-0-1 schema * require `tags` in grouped_metric + snowcat comment * fix formatting noise * update schema to make it valid * remove grouped_metrics from instance_stats schema * cljfmt appeasement * unbin cache_num_queries_cached value - alphabetize metrics generation * we can now guarantee metric values will be ints * jsonschema for instance_uuid, settings, and grouped_metrics * add analytics_uuid and make it required * lint + alphabetize instance stats json schema * update setting key type - add maxLength to some strings * lint jsonschema - Add description to setting.items.tags - add maxLength, and {min,max}imum to setting.items.value * Bump instance stats to 2-0-0 - remove analytics_uuid string length == 36 check - adds assertion to ensure required fields are set (and it passes) - adds info for embedding settings * adds a grouped-metric to stats ping - adds length info to the schema to pass jsonschema linting * cljfmt Co-authored-by: bryan <bryan.maass@gmail.com>

escherize added 3 commits October 8, 2024 11:25

adds data + schema for metrics stats ping

c8810ec

remove comment

b628c4a

annotate todos

b3f5ef3

escherize changed the title ~~adds data + schema for metrics stats ping~~ Adds metric snowplow data + schema for metrics Oct 8, 2024

metabase-bot bot assigned escherize Oct 8, 2024

metabase-bot bot added the .Team/AdminWebapp Admin and Webapp team label Oct 8, 2024

escherize added the no-backport Do not backport this PR to any branch label Oct 8, 2024

escherize added 4 commits October 8, 2024 13:21

fill in the rest of the metrics values

4118cd2

- and add defaults

fix some definitions + use a single timestamp

4d82879

shuffle stuff around to appease the namespace linter

a081ea4

- we aren't reaching into the api API, so the linter is more of a formality here.

Merge branch 'master' into add-metrics-to-snowplow-ping

a38fe1b

escherize marked this pull request as ready for review October 8, 2024 21:43

escherize requested a review from camsaul as a code owner October 8, 2024 21:43

escherize added 2 commits October 8, 2024 15:45

update docstring

07248f4

fix jsonschema, maybe

14b7a27

Somtom reviewed Oct 9, 2024

View reviewed changes

escherize added 4 commits October 9, 2024 10:46

review responses

0049acb

- revert changes to 1-0-0 - add metrics section to new file: 1-0-1 - bump ::instance_stats to "1-0-1" - add tags into 1-0-1 - make the code return tags (empty for now until we know what to tag things.) - also fix test that broke from shuffling settings around

remove a footgun

1ffd3b6

Merge branch 'master' into add-metrics-to-snowplow-ping

016b435

Merge branch 'add-metrics-to-snowplow-ping' of github.com:metabase/me…

9dda2c7

…tabase into add-metrics-to-snowplow-ping

luizarakaki reviewed Oct 9, 2024

View reviewed changes

snowplow/iglu-client-embedded/schemas/com.metabase/instance_stats/jsonschema/1-0-1 Outdated Show resolved Hide resolved

escherize added 4 commits October 9, 2024 14:46

add tags to metrics,

d68ccac

add grouped_metrics to jsonschema 1-0-1 format 1-0-1

indent

5e6672e

cljfmt

1326fd0

Merge branch 'master' into add-metrics-to-snowplow-ping

4e6afe0

escherize requested a review from noahmoss October 10, 2024 02:02

Somtom reviewed Oct 10, 2024

View reviewed changes

snowplow/iglu-client-embedded/schemas/com.metabase/instance_stats/jsonschema/1-0-1 Outdated Show resolved Hide resolved

snowplow/iglu-client-embedded/schemas/com.metabase/instance_stats/jsonschema/1-0-1 Outdated Show resolved Hide resolved

qnkhuat reviewed Oct 10, 2024

View reviewed changes

snowplow/iglu-client-embedded/schemas/com.metabase/instance_stats/jsonschema/1-0-1 Outdated Show resolved Hide resolved

escherize added 2 commits October 10, 2024 09:16

version should match filename

64cda71

Merge branch 'add-metrics-to-snowplow-ping' of github.com:metabase/me…

3cd6980

…tabase into add-metrics-to-snowplow-ping

noahmoss approved these changes Oct 11, 2024

View reviewed changes

escherize added 2 commits October 11, 2024 16:10

unbin cache_num_queries_cached value

35f5b7b

- alphabetize metrics generation

we can now guarantee metric values will be ints

7ff4ba7

escherize added the do not merge label Oct 14, 2024

escherize added 2 commits October 14, 2024 11:57

jsonschema for instance_uuid, settings, and grouped_metrics

f8397bb

add analytics_uuid and make it required

ddf7c64

escherize changed the title ~~Adds metric snowplow data + schema for metrics~~ Adds missing schemas + data for metrics to snowplow stats ping Oct 14, 2024

escherize added 3 commits October 14, 2024 13:24

lint + alphabetize instance stats json schema

19b4ff8

update setting key type

1961f50

- add maxLength to some strings

lint jsonschema

bdd3abe

- Add description to setting.items.tags - add maxLength, and {min,max}imum to setting.items.value

escherize removed the do not merge label Oct 14, 2024

Somtom reviewed Oct 15, 2024

View reviewed changes

snowplow/iglu-client-embedded/schemas/com.metabase/instance_stats/jsonschema/1-0-1 Outdated Show resolved Hide resolved

albertoperdomo requested changes Oct 15, 2024

View reviewed changes

src/metabase/analytics/stats.clj Show resolved Hide resolved

escherize added backport Automatically create PR on current release branch on merge and removed no-backport Do not backport this PR to any branch labels Oct 15, 2024

escherize requested a review from albertoperdomo October 15, 2024 15:33

escherize added 2 commits October 15, 2024 10:22

Bump instance stats to 2-0-0

fe1db2f

- remove analytics_uuid string length == 36 check - adds assertion to ensure required fields are set (and it passes) - adds info for embedding settings

adds a grouped-metric to stats ping

27e4e0e

- adds length info to the schema to pass jsonschema linting

cljfmt

df957a6

albertoperdomo approved these changes Oct 15, 2024

View reviewed changes

escherize merged commit e5c181b into master Oct 15, 2024
131 checks passed

escherize deleted the add-metrics-to-snowplow-ping branch October 15, 2024 20:47

github-automation-metabase mentioned this pull request Oct 15, 2024

🤖 backported "Adds missing schemas + data for metrics to snowplow stats ping" #48760

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds missing schemas + data for metrics to snowplow stats ping #48476

Adds missing schemas + data for metrics to snowplow stats ping #48476

escherize commented Oct 8, 2024 •

edited

Loading

Somtom left a comment

escherize commented Oct 11, 2024 •

edited

Loading

Somtom left a comment

albertoperdomo commented Oct 15, 2024

escherize commented Oct 15, 2024

escherize commented Oct 15, 2024

Adds missing schemas + data for metrics to snowplow stats ping #48476

Adds missing schemas + data for metrics to snowplow stats ping #48476

Conversation

escherize commented Oct 8, 2024 • edited Loading

Somtom left a comment

Choose a reason for hiding this comment

escherize commented Oct 11, 2024 • edited Loading

Open question:

Answered:

Somtom left a comment

Choose a reason for hiding this comment

albertoperdomo commented Oct 15, 2024

escherize commented Oct 15, 2024

escherize commented Oct 15, 2024

escherize commented Oct 8, 2024 •

edited

Loading

escherize commented Oct 11, 2024 •

edited

Loading