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.

Sign up for GitHub

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

Jump to bottom

[feat] Support Google Analytics 4 properties #1583

Closed
wants to merge 15 commits into from
Closed

[feat] Support Google Analytics 4 properties #1583

wants to merge 15 commits into from

Conversation

anikdhabal
Copy link
Contributor

Which problem is this PR solving?

Resolves #1332

Short description of the changes

Support Google Analytics 4 properties

@anikdhabal
updated to ga4
edce023 
Signed-off-by: Anik Dhabal Babu <adhabal2002@gmail.com>
@anikdhabal
Update ga.tsx
9b7d4f9 
Signed-off-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com>
@anikdhabal
Update utils.tsx
45dbdb2 
Signed-off-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com>
yurishkuro
yurishkuro previously approved these changes Jul 15, 2023
View reviewed changes
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

please remove package-lock.json from PR

package.json Outdated Show resolved Hide resolved
packages/jaeger-ui/src/utils/tracking/ga.tsx Show resolved Hide resolved
@anikdhabal
Delete package-lock.json
faa998c 
Signed-off-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com>
@anikdhabal
Update package.json
df87b38 
Signed-off-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com>
@anikdhabal
Update package.json
919c77f 
Signed-off-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com>
@anikdhabal
Update ga.tsx
4cad12e 
Signed-off-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com>
@anikdhabal
Update ga.tsx
351a29f 
Signed-off-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com>
yurishkuro
yurishkuro previously approved these changes Jul 16, 2023
View reviewed changes
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

lgtm. Did you test it with a real GA ID?

@anikdhabal
Copy link
Contributor Author

lgtm. Did you test it with a real GA ID?

Yes, it is fine.

@yurishkuro
Copy link
Member

Yes, it is fine.

can you include some screenshots?

@anikdhabal
Copy link
Contributor Author

Yes, it is fine.

can you include some screenshots?

Okk I will share some result.

@yurishkuro
Copy link
Member

@anikdhabal this PR does not include any configuration changes, so the same GA ID config will be used. Will it work if someone has the old GA ID in their config, which is not GA4?

@anikdhabal
Copy link
Contributor Author

@anikdhabal this PR does not include any configuration changes, so the same GA ID config will be used. Will it work if someone has the old GA ID in their config, which is not GA4?

No, it doesn't work for the old GA ID; it only works for GA4. I haven't found a straightforward way to support the old GA configuration. Let me do a bit more research about it or talk to the maintainer to include the configuration.

@yurishkuro
Copy link
Member

it only works for GA4. I haven't found a straightforward way to support the old GA configuration

Should we keep using both libraries and switch depending on the version of GA ID? Do you know what the timeline for Google to completely phase out old IDs?

@anshgoyalevil
Copy link
Member

According to Google,

Starting July 1, 2023, standard UA properties will stop processing data

@yurishkuro
Copy link
Member

Yeah, then I think it's fine to drop support for UA, and only support GA4

@anikdhabal
Copy link
Contributor Author

Yeah, then I think it's fine to drop support for UA, and only support GA4

Yeah it's fine.

@yurishkuro
Copy link
Member

CI is still failing on this pr

@anikdhabal
Copy link
Contributor Author

CI is still failing on this pr

I am fixing it now.

@yurishkuro
Copy link
Member

Fixed in #2071

@yurishkuro yurishkuro closed this Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yurishkuro yurishkuro yurishkuro left review comments

@everett980 everett980 Awaiting requested review from everett980

@rubenvp8510 rubenvp8510 Awaiting requested review from rubenvp8510

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature]: Support Google Analytics 4 properties
3 participants
@anikdhabal @yurishkuro @anshgoyalevil