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

add umami tracking codes via script tags #411

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Gregory-Pereira
Copy link
Collaborator

Relates to #281 .

Now that we have a prod deployment of Umami which created these tracking codes, we want to inject them as script tags into the codebase. I tested setting window.location.hostname manually in the console to one of our deployments, and it actually reroutes your page to that route, so you can't super create fake metrics which is good.

cc @nerdalert @vishnoianil

};

interface RootLayoutProps {
children: ReactNode;
}

const RootLayout = ({ children }: RootLayoutProps) => {
// Determine the environment based on hostname
Copy link
Member

Choose a reason for hiding this comment

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

I think rather than detecting whether it's prod or qa deployment in the code, we can simply provide the environment variables that takes the hostname, scriptsource and websiteId as an variable in .env. In the code it just use those values, irrespective of prod or qa deployment. Because these are basically day 0 configuration variables that user need to configure before the deployment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right so seems I forgot that we want to support local deployments with users injecting their own script tags - so we will need some env variable to track those for website-code and for script source. I dont think we want to set the Prod and QA values to be configurable though, otherwise we could get someone forwarding metrics from their local deployment to either one, so I think we should keep those checks in the source, and then add an option if its neither to check if the custom script tag and website-code are defined, and if so render those.

Copy link
Member

Choose a reason for hiding this comment

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

the challenge is, these URL can possibly change, for example we are planning to migrate ui.instructlab.ai to contribute.instructlab.ai. If that happens, we will have to make the code level changes to fix the umami metrics.

How about we have a environment variable DEPLOYMENT_ENVIRONMENT that takes two value prod and qa, and use that to detect the deployment environment. If you see concern with that, we can go with checking the url itself.

Copy link
Collaborator Author

@Gregory-Pereira Gregory-Pereira Dec 15, 2024

Choose a reason for hiding this comment

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

What about we define those URLs in values in src/types/const.ts? That way we can include it in source, so we only have to update 1 place, and we import the constants file?

If you would prefer the above implementation I am happy to go with that, just wanted your feedback on what you think would be a better solution here.

@Gregory-Pereira Gregory-Pereira force-pushed the implement-umami-tracking-codes-in-typescript branch 2 times, most recently from 5e5c51f to 5e8d697 Compare December 15, 2024 18:27
@Gregory-Pereira Gregory-Pereira force-pushed the implement-umami-tracking-codes-in-typescript branch from 5e8d697 to d7be59a Compare December 15, 2024 18:32
@Gregory-Pereira
Copy link
Collaborator Author

This does seem to work, prior to adding the 2nd commit I was seeing metrics show up for my local deployment:
Screenshot 2024-12-15 at 10 21 05 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metrics Related to telemetry ready-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants