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

feat: scene-load transctions based on SceneManagerAPI #768

Merged
merged 42 commits into from
Sep 29, 2022

Conversation

vaind
Copy link
Collaborator

@vaind vaind commented May 27, 2022

See #235

This PR uses SceneManagerAPI which is only available in Unity 2020+

Samples:https://sentry.io/organizations/sentry-sdks/performance/sentry-unity:4e44bacb819a4509ac4bb20b2c53d154/?project=5439417&query=transaction.duration%3A%3C15m&statsPeriod=30d&transaction=unity.runtime.startup&unselectedSeries=p100%28%29

  • move to SentryInitialization.cs (or use reflection?)
  • figure out if we can add the integration before the first scene starts loading

The goal is to create a Transaction and a Span for loading scenes. These will be further extended by future auto instrumentation.
Screenshot 2022-09-28 at 15 10 29

@github-actions
Copy link
Contributor

github-actions bot commented May 27, 2022

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against ac193c1

package-dev/Runtime/SentryIntegrations.cs Outdated Show resolved Hide resolved
package-dev/Runtime/SentryIntegrations.cs Outdated Show resolved Hide resolved
package-dev/Runtime/SentryIntegrations.cs Outdated Show resolved Hide resolved
package-dev/Runtime/SentryIntegrations.cs Outdated Show resolved Hide resolved
package-dev/Runtime/SentryIntegrations.cs Outdated Show resolved Hide resolved
@bitsandfoxes bitsandfoxes marked this pull request as ready for review September 27, 2022 13:09

if (GetTestArg() == null)
{
Debug.Log("SmokeTester.Configure() called but skipped because this is not a SmokeTest (no arg)");
Copy link
Contributor

Choose a reason for hiding this comment

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

A remnant from having the smoke test as part of the sample project.

Copy link
Collaborator Author

@vaind vaind left a comment

Choose a reason for hiding this comment

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

package-dev/Runtime/SentryIntegrations.cs Show resolved Hide resolved
package-dev/Tests/Runtime/SentryIntegrationsTests.cs Outdated Show resolved Hide resolved
test/Scripts.Integration.Test/Editor/Builder.cs Outdated Show resolved Hide resolved
test/Scripts.Integration.Test/integration-test.ps1 Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@bitsandfoxes
Copy link
Contributor

Maybe drop the "unity." prefix? Also, I've noticed the spans are missing names - what's the best practice in other SDKs?

https://sentry.io/organizations/sentry-sdks/performance/sentry-unity:4e44bacb819a4509ac4bb20b2c53d154/?project=5439417&query=transaction.duration%3A%3C15m&statsPeriod=30d&transaction=unity.runtime.startup&unselectedSeries=p100%28%29 image

image

Thanks for pointing that out.

Initially, I was a bit confused about the naming/description/operation of transactions and spans but the team came up with a scheme I think we can go with for now.

@bitsandfoxes bitsandfoxes merged commit 0cbc397 into main Sep 29, 2022
@bitsandfoxes bitsandfoxes deleted the feat/scene-load-tx branch September 29, 2022 17:04
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.

3 participants