-
Notifications
You must be signed in to change notification settings - Fork 7
feat(glean): enable glean in prod - SOCWEB-145 #92
Conversation
@wtfluckey That should work! As long as we can filter out dev events, I see no harm in triggering them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @wtfluckey ! Looks good an interested in hearing more about the path syntax you're using!
import { userAgent } from '../../../telemetry/generated/identifiers' | ||
import { engagement } from '../../../telemetry/generated/ui' | ||
import { engagementDetails } from '../../../telemetry/engagementDetails' | ||
import { userAgent } from '~~/telemetry/generated/identifiers' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the ~~
syntax is new and exciting to me! Are you able to share documentation on how this works? Does it work in all environments? Unix/Windows etc?
If this is a home path, can we be sure it leads to the same place, vs traversing relatively?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a fun one to track down! It's built into Nuxt actually: https://nuxt.com/docs/api/nuxt-config#alias. It looks like the ~~
will always lead to /<rootDir>
@@ -13,9 +13,11 @@ export default defineNuxtPlugin((nuxtApp) => { | |||
const GLEAN_APP_ID = 'moso-mastodon-web' | |||
const env = useAppConfig().env | |||
const devMode = env === ('dev' || 'canary' || 'preview') | |||
const prodMode = env === 'release' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry I may be out of the loop on this; does useAppConfig().env
distinguish between stage and prod? What is the env value for stage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question @aaga! I assumed that it was able to distinguish between stage & production, but I haven't verified that yet. I reached out to Kirill to confirm whether his team is seeing events coming from the dev
channel, and if so, he should be able to tell us which if that env
is getting set for stage as well.
In the meantime, I'm also looking into useBuildInfo()
and whether that might be what we want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did some digging here and it looks like useBuildInfo
just uses useAppConfig
under the hood, so I'm not feeling super confident that either of these work the way I think they do. In order to help test this, I've removed the environment check when initializing glean, but passing through the env
that gets set from useBuildInfo
as the channel
- this will allow us to learn from the data team what that env
variable is getting set as in staging & production. After that we can add back in the environment check if we want to.
@@ -12,7 +12,6 @@ useHydratedHead({ | |||
const route = useRoute() | |||
|
|||
const isRootPath = computedEager(() => route.name === 'settings') | |||
const devMode = useAppConfig().env === ('dev' || 'canary' || 'preview') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const env = useBuildInfo().env | ||
const devMode = env === 'dev' | ||
const userSettings = useUserSettings() | ||
const allowGlean = getPreferences(userSettings.value, 'allowGlean') | ||
const uploadEnabled = devMode && allowGlean | ||
const userAllowGlean = getPreferences(userSettings.value, 'allowGlean') | ||
const uploadEnabled = userAllowGlean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed the environment check from uploadEnabled
because I'm not confident that stage and production will get represented correctly from either useBuildInfo
or useAppConfig
. In order to test this and help us understand it better, I'm passing the channel: env
when initializing Glean - this will allow us to find out what that env gets set as in the other environments.
fbcc08d
to
90ef3ca
Compare
Goal
Enable Glean in production so we can start tracking analytics
SOCWEB-145
Implementation Details
Added a new variableconst gleanEnabled = (devMode || prodMode)
Use newgleanEnabled
variable when initializing Gleanenv
to useuseBuildInfo
rather thanuseAppConfig
(not super confident that either of these work the way I think they do)uploadEnabled
allowGlean
touserAllowGlean
to be a little more specific../../../
Question
Right now, this enables Glean for all environments (dev, staging and production). @kirill-demtchouk you should be able to filter the events based on environment via the
channel
that we send through, but I am open to adjusting this so it's not uploading events for all environments. Let me know your thoughts!