-
-
Notifications
You must be signed in to change notification settings - Fork 38
feat(auto-install): Fail the build on OpenTelemetry version downgrades #1350
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
base: side-quest/sagp-bom-version-use-skill-create
Are you sure you want to change the base?
Changes from 6 commits
44f8dce
2d7a009
48c629f
a8bdce3
0d0930a
e4f8f9b
27a5acf
a039210
045ea98
dbec0c2
986b609
5cbd829
6a5a723
bb20adc
45dd618
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,238 @@ | ||||||||||
| package io.sentry.android.gradle.tasks | ||||||||||
|
|
||||||||||
| import io.sentry.android.gradle.extensions.SentryPluginExtension | ||||||||||
| import io.sentry.android.gradle.telemetry.SentryTelemetryService | ||||||||||
| import io.sentry.android.gradle.telemetry.withSentryTelemetry | ||||||||||
| import io.sentry.android.gradle.util.SemVer | ||||||||||
| import org.gradle.api.DefaultTask | ||||||||||
| import org.gradle.api.GradleException | ||||||||||
| import org.gradle.api.Project | ||||||||||
| import org.gradle.api.artifacts.component.ModuleComponentSelector | ||||||||||
| import org.gradle.api.artifacts.result.ResolvedComponentResult | ||||||||||
| import org.gradle.api.artifacts.result.ResolvedDependencyResult | ||||||||||
| import org.gradle.api.provider.Property | ||||||||||
| import org.gradle.api.provider.Provider | ||||||||||
| import org.gradle.api.tasks.Input | ||||||||||
| import org.gradle.api.tasks.Internal | ||||||||||
| import org.gradle.api.tasks.TaskAction | ||||||||||
| import org.gradle.api.tasks.TaskProvider | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Fails the build when the OpenTelemetry versions resolved on the runtime classpath were downgraded | ||||||||||
| * below what the Sentry OpenTelemetry integration requires. | ||||||||||
| * | ||||||||||
| * Sentry's `sentry-opentelemetry-*` artifacts declare the exact OpenTelemetry versions they were | ||||||||||
| * built and tested against. When another dependency management mechanism (most commonly | ||||||||||
| * `io.spring.dependency-management` / the Spring Boot BOM) forces a lower OpenTelemetry version, | ||||||||||
| * running against those downgraded versions can cause `ClassNotFoundException` / | ||||||||||
| * `NoSuchMethodError` at runtime. We detect that downgrade here and fail fast with actionable | ||||||||||
| * guidance instead of letting it blow up at runtime. | ||||||||||
| */ | ||||||||||
| abstract class SentryOpenTelemetryVersionCheckTask : DefaultTask() { | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This task is missing either a In this case since we have no outputs it should probably be
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Skipping the output file for now. We can always add that later if needed. |
||||||||||
|
|
||||||||||
| @get:Internal abstract val rootComponent: Property<ResolvedComponentResult> | ||||||||||
|
|
||||||||||
| @get:Input abstract val docsUrl: Property<String> | ||||||||||
|
|
||||||||||
| @get:Input abstract val springDependencyManagementApplied: Property<Boolean> | ||||||||||
|
|
||||||||||
| @get:Input abstract val hasSentryOpenTelemetryDependency: Property<Boolean> | ||||||||||
|
|
||||||||||
| @get:Input abstract val verifyEnabled: Property<Boolean> | ||||||||||
|
|
||||||||||
| init { | ||||||||||
| description = | ||||||||||
| "Checks that the resolved OpenTelemetry versions are compatible with the Sentry " + | ||||||||||
| "OpenTelemetry integration" | ||||||||||
| group = "verification" | ||||||||||
| // Reference task-owned properties only (not the extension/project) so the task stays | ||||||||||
| // configuration-cache compatible. | ||||||||||
| @Suppress("LeakingThis") | ||||||||||
| onlyIf { verifyEnabled.get() && hasSentryOpenTelemetryDependency.get() } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| @TaskAction | ||||||||||
| fun action() { | ||||||||||
| val mismatches = collectDowngrades(rootComponent.get()) | ||||||||||
| if (mismatches.isNotEmpty()) { | ||||||||||
| throw GradleException( | ||||||||||
| buildMessage(mismatches, docsUrl.get(), springDependencyManagementApplied.get()) | ||||||||||
| ) | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| companion object { | ||||||||||
| private const val SENTRY_GROUP = "io.sentry" | ||||||||||
| private const val SENTRY_OPENTELEMETRY_ARTIFACT_PREFIX = "sentry-opentelemetry-" | ||||||||||
| private const val OTEL_GROUP = "io.opentelemetry" | ||||||||||
| private const val SPRING_DEPENDENCY_MANAGEMENT_PLUGIN_ID = "io.spring.dependency-management" | ||||||||||
|
|
||||||||||
| internal data class VersionMismatch( | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: In this case we only ever hold downgrades so this might be a better name:
Suggested change
|
||||||||||
| val module: String, | ||||||||||
| val requested: String, | ||||||||||
| val resolved: String, | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can get more information from Gradle's dependency management engine in order to improve the error message.
Suggested change
Set Let me know if you need more info here! |
||||||||||
| ) | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Walks the resolved dependency graph and returns, for every OpenTelemetry module that a | ||||||||||
| * `sentry-opentelemetry-*` artifact depends on, the ones whose resolved version is lower than | ||||||||||
| * the version Sentry requested. | ||||||||||
| */ | ||||||||||
| internal fun collectDowngrades(root: ResolvedComponentResult): List<VersionMismatch> { | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you think it would be possible to split this pure logic from the gradle task and then unit test that separately? |
||||||||||
| val mismatches = linkedMapOf<String, VersionMismatch>() | ||||||||||
| val visited = mutableSetOf<Any>() | ||||||||||
| val stack = ArrayDeque<ResolvedComponentResult>() | ||||||||||
| stack.addLast(root) | ||||||||||
| while (stack.isNotEmpty()) { | ||||||||||
| val component = stack.removeLast() | ||||||||||
| if (!visited.add(component.id)) { | ||||||||||
| continue | ||||||||||
| } | ||||||||||
| val fromSentryOpenTelemetry = component.isSentryOpenTelemetryArtifact() | ||||||||||
| for (dependency in component.dependencies) { | ||||||||||
| if (dependency !is ResolvedDependencyResult) { | ||||||||||
| continue | ||||||||||
| } | ||||||||||
| if (fromSentryOpenTelemetry) { | ||||||||||
| val requested = dependency.requested | ||||||||||
| if (requested is ModuleComponentSelector && requested.isOpenTelemetry()) { | ||||||||||
| val resolvedVersion = dependency.selected.moduleVersion?.version | ||||||||||
| if (resolvedVersion != null && isDowngrade(requested.version, resolvedVersion)) { | ||||||||||
| val module = "${requested.group}:${requested.module}" | ||||||||||
| mismatches.putIfAbsent( | ||||||||||
| module, | ||||||||||
| VersionMismatch(module, requested.version, resolvedVersion), | ||||||||||
| ) | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| stack.addLast(dependency.selected) | ||||||||||
| } | ||||||||||
| } | ||||||||||
| return mismatches.values.toList() | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private fun ResolvedComponentResult.isSentryOpenTelemetryArtifact(): Boolean { | ||||||||||
| val moduleVersion = moduleVersion ?: return false | ||||||||||
| return moduleVersion.group == SENTRY_GROUP && | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this logic is repeated on line 229. would it be worth making a function? |
||||||||||
| moduleVersion.name.startsWith(SENTRY_OPENTELEMETRY_ARTIFACT_PREFIX) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Matches every OpenTelemetry group (io.opentelemetry, io.opentelemetry.instrumentation, | ||||||||||
| // io.opentelemetry.semconv, ...) so any OTel module a sentry-opentelemetry-* artifact declares | ||||||||||
| // is covered as that set grows. We only ever inspect edges declared by Sentry's own artifacts, | ||||||||||
| // so this never flags unrelated OpenTelemetry usage. | ||||||||||
| private fun ModuleComponentSelector.isOpenTelemetry(): Boolean = | ||||||||||
| group == OTEL_GROUP || group.startsWith("$OTEL_GROUP.") | ||||||||||
|
|
||||||||||
| private fun isDowngrade(requested: String, resolved: String): Boolean { | ||||||||||
| // A blank requested version means it was declared without a concrete version (e.g. managed | ||||||||||
| // elsewhere); there is nothing to compare against, so don't flag it. | ||||||||||
| if (requested.isBlank() || requested == resolved) { | ||||||||||
| return false | ||||||||||
| } | ||||||||||
| return try { | ||||||||||
| SemVer.parse(resolved) < SemVer.parse(requested) | ||||||||||
| } catch (e: IllegalArgumentException) { | ||||||||||
| // Non-semver version strings can't be compared reliably; err on the side of not failing. | ||||||||||
| false | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| internal fun buildMessage( | ||||||||||
| mismatches: List<VersionMismatch>, | ||||||||||
| docsUrl: String, | ||||||||||
| springDependencyManagementApplied: Boolean, | ||||||||||
| ): String { | ||||||||||
| val details = | ||||||||||
| mismatches.joinToString(separator = "\n") { mismatch -> | ||||||||||
| " - ${mismatch.module}: Sentry requires ${mismatch.requested} " + | ||||||||||
| "but ${mismatch.resolved} was resolved" | ||||||||||
| } | ||||||||||
| val fix = | ||||||||||
| if (springDependencyManagementApplied) { | ||||||||||
| // io.spring.dependency-management enforces its managed versions via a resolution strategy | ||||||||||
| // that overrides Gradle platform() constraints, so the BOM has to be imported through the | ||||||||||
| // plugin's own API to win. | ||||||||||
| """ | ||||||||||
| |To fix this, import the Sentry OpenTelemetry BOM through io.spring.dependency-management so | ||||||||||
| |its versions win dependency resolution: | ||||||||||
| | | ||||||||||
| | dependencyManagement { | ||||||||||
| | imports { | ||||||||||
| | mavenBom("io.sentry:sentry-opentelemetry-bom:<sentryVersion>") | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would it make sense to fill in from the value in and also for 174? |
||||||||||
| | } | ||||||||||
| | } | ||||||||||
| """ | ||||||||||
| .trimMargin() | ||||||||||
| } else { | ||||||||||
| """ | ||||||||||
| |To fix this, add the Sentry OpenTelemetry BOM as a platform dependency so its versions win | ||||||||||
| |dependency resolution: | ||||||||||
| | | ||||||||||
| | dependencies { | ||||||||||
| | implementation(platform("io.sentry:sentry-opentelemetry-bom:<sentryVersion>")) | ||||||||||
|
cursor[bot] marked this conversation as resolved.
Outdated
|
||||||||||
| | } | ||||||||||
| """ | ||||||||||
| .trimMargin() | ||||||||||
| } | ||||||||||
| return """ | ||||||||||
| |Sentry detected that OpenTelemetry was downgraded below the version its integration requires. | ||||||||||
| | | ||||||||||
| |The Sentry OpenTelemetry integration was built against specific OpenTelemetry versions, | ||||||||||
| |but the following were downgraded by another dependency management mechanism: | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can also add that users can run this command or check a build scan for more details:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm afraid of getting the command wrong and adding even more confusion. Also not fully sure if this works for Spring Boot so I'd rather skip this for now. |
||||||||||
| | | ||||||||||
| |$details | ||||||||||
| | | ||||||||||
| |Running with these downgraded OpenTelemetry versions can cause | ||||||||||
| |ClassNotFoundException / NoSuchMethodError at runtime. | ||||||||||
| | | ||||||||||
| |$fix | ||||||||||
| | | ||||||||||
| |See $docsUrl for details. | ||||||||||
| | | ||||||||||
| |You can disable this check with: | ||||||||||
| | | ||||||||||
| | sentry { | ||||||||||
| | autoInstallation.verifyOpenTelemetryVersions = false | ||||||||||
| | } | ||||||||||
| """ | ||||||||||
| .trimMargin() | ||||||||||
| } | ||||||||||
|
|
||||||||||
| fun register( | ||||||||||
| project: Project, | ||||||||||
| extension: SentryPluginExtension, | ||||||||||
| sentryTelemetryProvider: Provider<SentryTelemetryService>, | ||||||||||
| configurationName: String, | ||||||||||
| docsUrl: String, | ||||||||||
| ): TaskProvider<SentryOpenTelemetryVersionCheckTask> { | ||||||||||
| return project.tasks.register( | ||||||||||
| "verifySentryOpenTelemetryVersions", | ||||||||||
| SentryOpenTelemetryVersionCheckTask::class.java, | ||||||||||
| ) { task -> | ||||||||||
| val configuration = project.configurations.getByName(configurationName) | ||||||||||
| task.rootComponent.set(configuration.incoming.resolutionResult.rootComponent) | ||||||||||
| task.docsUrl.set(docsUrl) | ||||||||||
| // Resolved lazily (at end of configuration) so it reflects the final plugin set regardless | ||||||||||
| // of the order plugins are applied in the consumer's build. | ||||||||||
| task.springDependencyManagementApplied.set( | ||||||||||
| project.provider { | ||||||||||
| project.pluginManager.hasPlugin(SPRING_DEPENDENCY_MANAGEMENT_PLUGIN_ID) | ||||||||||
| } | ||||||||||
| ) | ||||||||||
| // Cheap declared-dependency check (no graph resolution) so we skip entirely for the common | ||||||||||
| // case of projects that don't use Sentry OpenTelemetry at all. | ||||||||||
| task.hasSentryOpenTelemetryDependency.set( | ||||||||||
| project.provider { | ||||||||||
| configuration.allDependencies.any { | ||||||||||
| it.group == SENTRY_GROUP && it.name.startsWith(SENTRY_OPENTELEMETRY_ARTIFACT_PREFIX) | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
cursor[bot] marked this conversation as resolved.
|
||||||||||
| ) | ||||||||||
| task.verifyEnabled.set(extension.autoInstallation.verifyOpenTelemetryVersions) | ||||||||||
| task.withSentryTelemetry(extension, sentryTelemetryProvider) | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import io.sentry.android.gradle.extensions.SentryPluginExtension | |
| import io.sentry.android.gradle.sourcecontext.OutputPaths | ||
| import io.sentry.android.gradle.sourcecontext.SourceContext | ||
| import io.sentry.android.gradle.tasks.SentryGenerateDebugMetaPropertiesTask | ||
| import io.sentry.android.gradle.tasks.SentryOpenTelemetryVersionCheckTask | ||
| import io.sentry.android.gradle.tasks.dependencies.SentryExternalDependenciesReportTaskV2 | ||
| import io.sentry.android.gradle.telemetry.SentryTelemetryService | ||
| import io.sentry.android.gradle.util.SentryPluginUtils | ||
|
|
@@ -130,6 +131,24 @@ constructor(private val buildEvents: BuildEventListenerRegistryInternal) : Plugi | |
| } | ||
|
|
||
| project.installDependencies(extension, false) | ||
|
|
||
| val verifyOpenTelemetryVersionsTask = | ||
| SentryOpenTelemetryVersionCheckTask.register( | ||
| project = project, | ||
| extension = extension, | ||
| sentryTelemetryProvider = sentryTelemetryProvider, | ||
| configurationName = "runtimeClasspath", | ||
| docsUrl = OPENTELEMETRY_VERSION_MISMATCH_DOCS_URL, | ||
| ) | ||
| // Gate compilation so any build that compiles, packages, runs or tests (assemble, bootJar, | ||
| // bootRun, check, ...) fails fast on an OpenTelemetry version mismatch. | ||
| project.tasks.named("classes").configure { it.dependsOn(verifyOpenTelemetryVersionsTask) } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verify hook skips test buildsHigh Severity The OpenTelemetry verify task is attached only as a dependency of Reviewed by Cursor Bugbot for commit e4f8f9b. Configure here. |
||
| } | ||
| } | ||
|
|
||
| companion object { | ||
| // TODO: finalize the troubleshooting docs URL before release. | ||
| internal const val OPENTELEMETRY_VERSION_MISMATCH_DOCS_URL = | ||
| "https://docs.sentry.io/platforms/java/opentelemetry/troubleshooting/" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO docs don't exist yet
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PR is up: getsentry/sentry-docs#18647 |
||
| } | ||
| } | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this isn't tied to auto install, the option shouldn't live under auto install.