Skip to content

Update Analytics to use new API #6221

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

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

Update Analytics to use new API #6221

wants to merge 12 commits into from

Conversation

maryamziaa
Copy link
Contributor

@maryamziaa maryamziaa commented Jul 16, 2025

Proposed change(s)

The old analytics API has been deprecated. This PR updates the analytics implementation to use the new API.

Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)

Types of change(s)

  • Bug fix
  • New feature
  • Code refactor
  • Breaking change
  • Documentation update
  • Other (please describe): Upgraded to new Analytics API

Checklist

  • Added tests that prove my fix is effective or that my feature works
  • Updated the changelog (if applicable)
  • Updated the documentation (if applicable)
  • Updated the migration guide (if applicable)

Other comments

@@ -26,6 +28,8 @@ jobs:
- uses: actions/setup-dotnet@v4
with:
dotnet-version: '8.0.202'
- name: Clean dotnet shared memory
run: sudo rm -rf /tmp/.dotnet/shm
Copy link
Contributor Author

Choose a reason for hiding this comment

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

any objections to do a cleanup first? Anything else is using /tmp/.dotnet/shm?

@@ -90,7 +51,7 @@ internal static bool EnableAnalytics()
s_TrainingSessionGuid = Guid.NewGuid();
}

return s_EventsRegistered;
return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

EnableAnalytics() always returns true if the conditions apply, i.e. UNITY_EDITOR && MLA_UNITY_ANALYTICS_MODULE && ENABLE_CLOUD_SERVICES_ANALYTICS. Is that OK? We don't RegisterEvent anymore in the new API.

{
s_SentModels = new HashSet<ModelAsset>();
}

return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

EnableAnalytics() always returns true if the conditions apply, i.e. UNITY_EDITOR && MLA_UNITY_ANALYTICS_MODULE && ENABLE_CLOUD_SERVICES_ANALYTICS. Is that OK? We don't RegisterEvent anymore in the new API.

@maryamziaa maryamziaa requested a review from a team July 17, 2025 21:57
Copy link

@antoine-agthe-unity antoine-agthe-unity left a comment

Choose a reason for hiding this comment

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

I invite you to give more explanation about why IAnalytics and IData types are implemented that way as it might look exotic, who an external point of view.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants