feat: remove ReactGA and migrate to GA4 for tracking
#2071
Conversation
Signed-off-by: Eshaan Aggarwal <96648934+EshaanAgg@users.noreply.github.com>
Signed-off-by: Eshaan Aggarwal <96648934+EshaanAgg@users.noreply.github.com>
Signed-off-by: Eshaan Aggarwal <96648934+EshaanAgg@users.noreply.github.com>
EshaanAgg
changed the title
ReactGA and migrate to GA4 for tracking
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2071 +/- ##
==========================================
+ Coverage 96.56% 96.57% +0.01%
==========================================
Files 255 254 -1
Lines 7623 7626 +3
Branches 1985 1986 +1
==========================================
+ Hits 7361 7365 +4
+ Misses 262 261 -1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Eshaan Aggarwal <96648934+EshaanAgg@users.noreply.github.com>
| import ReactGA from 'react-ga'; | ||
| import { WindowWithGATracking } from './ga'; | ||
|
|
||
| declare let window: WindowWithGATracking; | ||
|
|
||
| // eslint-disable-next-line import/prefer-default-export | ||
| export const logTrackingCalls = () => { |
There was a problem hiding this comment.
can we move this function inside ga.tsx (and tests)? I don't see it being used outside, so odd to expose as public, especially since the dataLayer contact is for GA only.
There was a problem hiding this comment.
in fact, if feels like it could be combined with the gtag() function instead of always calling it explicitly
There was a problem hiding this comment.
Aight. The combining idea sounds nice. I'll check it out.
There was a problem hiding this comment.
I have combined the logging with the gtag function itself in the latest implementation. This WindowWithGATracking is now only defined and used in ga.tsx
packages/jaeger-ui/src/vite-env.d.ts
Outdated
| // defined in utils/tracking/ga.ts | ||
| dataLayer: never; | ||
| } | ||
| } |
There was a problem hiding this comment.
Could you elaborate why this change is needed?
There was a problem hiding this comment.
On second thoughts, the most ideal implementation would be for
dataLayerto have the appropriate types withinga.tsand have the typeneveroutside it so that no other module tries to access or modify the same.
This had been my initial reasoning but I think it would be better to remove it considering the way you have advised to handle the types in the previous comment.
yurishkuro
added
the
changelog:bugfix-or-minor-feature
Signed-off-by: Eshaan Aggarwal <96648934+EshaanAgg@users.noreply.github.com>
Which problem is this PR solving?
Fixes #1332
Description of the changes
ReactGAlibrary for analytics and instead load the script ourselves asynchronouslygtagfunction with the appropriate parametersHow was this change tested?
Checklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test