getsentry / sentry

Developer-first error tracking and performance monitoring
https://sentry.io
Other
39.08k stars 4.2k forks source link

Releases coming from Android are not shown in releases overview even though same package is used #76502

Open Angelodaniel opened 2 months ago

Angelodaniel commented 2 months ago

Environment

SaaS (https://sentry.io/)

Steps to Reproduce

Expected Result

Both iOS and Android releases should be surfaced when using the release.package filter with com.x. (full link in Jira) Ideally I want to use the Releases API but those are related

Actual Result

Only iOS releases are returned, while Android releases are not surfaced even though they exist.

Product Area

Releases

Link

See Jira

DSN

No response

Version

No response

┆Issue is synchronized with this Jira Improvement by Unito

getsantry[bot] commented 2 months ago

Assigning to @getsentry/support for routing ⏲️

getsantry[bot] commented 2 months ago

Routing to @getsentry/product-owners-releases for triage ⏲️

bruno-garcia commented 1 month ago

@romtsn before we look into the backend for this, anything on the Android SDK we're missing? do we send release.package ? I'm actually not familiar with when/where this comes from

romtsn commented 1 month ago

@bruno-garcia I've taken a look, and it seems like there's a subtle difference in the releases between iOS and Android

Android: com.x.y.z@v4.x.x+787 iOS: com.x.y.z@4.x.x+569

Note the prepending v part in the Android release. This v comes from the customer and we just read it out as-is. This probably messes up our release parsing logic.

I guess we should either recommend them to use a proper semver for their versionName, or we should handle this case and sanitize it on the backend side to comply with Sentry’s expectations and just have digits there.

We could also sanitize it only in the Android SDK, but then this issue may come up elsewhere.

bruno-garcia commented 1 month ago

There'll be a ton of apps already out there with the v prefix, and it's pretty common to do that.

@c298lee how challenging would it be to support the v prefix?

c298lee commented 1 month ago

@bruno-garcia to support the v prefix, we would need to change the release parser in Relay. We would need to modify the regex to accept v. If this is what we want to do, do we only want to add support for the v prefix? I'm not very familiar with release versions, so I'm not sure if other prefixes are common too.

bruno-garcia commented 1 month ago

I've seen quite a few projects prefixing releases with v but I can't think of other very common practicies like that to call out right now. So I'd say the v support should be enough.

We do need to think about how this affects existing data. The regex is only used in search? Or do we have some side effect of that that could result in an existing release in Sentry forking as a new one?

c298lee commented 1 month ago

The current issue is that the package does not exist in Redash, which is why it's not showing up in searches. Currently, any release that is not a semver or commit SHA could only be searched using the release keyword because the release.package, release.version, and release.build are not populated when a new release is added.

So while the release isn't parsed as semver due to the v prefix (not valid semver), the user should be able to search by using a wild card like: release:"app@*1.0.0" so including the data with release prefixed with v, and those without it.

We could change the regex in Sentry to allow the version to be prefixed by v as suggested above, treating them as semver. That would fill out the package, major, minor, patch, revision etc. But I'm not sure about the full implications of this, and it's likely it would cause other issues.

Since we are not sure about the full consequences of doing this, we should recommend that they remove the v so that their release is semver compatible, as recommended by our docs. In the future, we could look into how many projects have the prefixed v in their version to determine the scope of this issue and do a deeper analysis in all of Sentry's release support, in order to determine the impact of this change.

c298lee commented 1 month ago

We'll normalize and sanitize the release string as far as we can do on the sdk's side, but if it's still invalid (in our terms) after that we'll print a warning/throw an error that the releases feature won't work as expected. This would be done in this ticket: https://github.com/getsentry/sentry-java/issues/3700

For now the customer can mitigate this in future releases by setting the release string themselves:

SentryAndroid.init(context) { options ->
  val packageInfo = ... // get the package info from context
  options.release = packageInfo.packageName + "@" + packageInfo.versionName.substringAfter('v') + "+" + versionCode
}