Add a task to compare public APIs for every stable module. #3183

jkwatson · 2021-04-27T20:46:22Z

This adds a japicmp task that will generate a diff of public APIs. By default, it will generate the current local version vs. the latest released version. You can also specify specific versions to compare and generate diffs for with two gradle properties:

For example
./gradlew japicmp -PapiBaseVersion=1.0.0 -PapiNewVersion=1.1.0
will generate diffs between 1.1.0 and 1.0.0.

I would like every PR to include updates to the diff files (if any), but I'm not sure the best way to enforce that.

I would also like, before every release, a maintainer generate the diffs vs. the previous release and make sure that there are no surprise API changes or breakages.

As a baseline, I have generated diffs between 1.1.0 and 1.0.0, for historical completeness, as well as the current diffs vs. the latest.

This code is pretty hacky, so suggestions for improvement are very welcome!

resolves #2692

jkwatson · 2021-04-27T20:49:37Z

exporters/zipkin/docs/api_diff_1.1.0_vs_1.0.0.txt

+Comparing source compatibility of  against 
+***  MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.exporter.zipkin.ZipkinSpanExporter  (not serializable)
+	===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+	+++  NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.util.logging.Logger baseLogger


this is a bummer that I found while working on this. And, why I think we should make something like this check a part of our normal process.

Totally agree with above statement.

codecov · 2021-04-27T20:58:57Z

Codecov Report

Merging #3183 (8901269) into main (910e66c) will not change coverage.
The diff coverage is n/a.

❗ Current head 8901269 differs from pull request most recent head a741131. Consider uploading reports for the commit a741131 to get more accurate results

@@     Coverage Diff      @@
##   main   #3183   +/-   ##
============================
============================

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 910e66c...a741131. Read the comment docs.

jkwatson · 2021-04-27T21:27:38Z

api/all/docs/api_diff_1.1.0_vs_1.0.0.txt

@@ -0,0 +1,2 @@
+Comparing source compatibility of  against 


Note: the lack of versions in the file appears to be due to this bug: melix/japicmp-gradle-plugin#31

I guess in the meantime it wouldn't be that hard for us to overwrite the file with a line that has an appropriate version if we want to. But no big deal

definitely something we could enhance ourselves with a little sed or something.

jkwatson · 2021-04-27T21:47:53Z

Also note that this picked up the API change from #2799 after I rebased. See commit 891b1ad

build.gradle.kts

anuraaga · 2021-04-28T04:34:46Z

api/all/docs/api_diff_1.1.0_vs_1.0.0.txt

@@ -0,0 +1,2 @@
+Comparing source compatibility of  against 


I think it'd be better to aggregate all these into top level, docs/${newVersion}/${archivesBaseName} to get an idea of all the changes in a version more easily.

yeah, at one point I had this implemented and decided it would be better to keep the diffs close to the source, but I don't feel super strongly about that.

Also, I did want to make sure that we saw diffs vs. the latest release. what directory would that end up in, do you think? Or, would we just put the concrete ones up at the top level for the historical records, and put the incremental snapshot changes inside the modules?

I think I'd still be find with diff vs current (docs/current/) to be aggregated, and we get to see all the new API in the next version very easily.

Both of my suggestions should have been docs/apidiffs by the way :)

cool. I'll update this to put everything under docs/apidiffs:

docs/apidiffs/current/<module>.txt
docs/apidiffs/v1.1.0_v1.0.0/<module>.txt

ok, moved and regenerated. please take a look.

build.gradle.kts

ghost

Great stuff, I'd really like to see that in instrumentation as well.

ghost · 2021-04-29T17:47:27Z

context/src/main/java/io/opentelemetry/context/Context.java

@@ -108,6 +108,8 @@ static Context root() {
   * during an invocation to another thread. For example, you may use something like {@code Executor
   * dbExecutor = Context.wrapTasks(threadPool)} to ensure calls like {@code dbExecutor.execute(()
   * -> database.query())} have {@link Context} available on the thread executing database queries.
+   *
+   * @since 1.1.0


Nice! Shows that the tool can be very useful also for documentation purposes.

ghost · 2021-04-29T17:48:11Z

exporters/zipkin/docs/api_diff_1.1.0_vs_1.0.0.txt

+Comparing source compatibility of  against 
+***  MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.exporter.zipkin.ZipkinSpanExporter  (not serializable)
+	===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+	+++  NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.util.logging.Logger baseLogger


Totally agree with above statement.

ocelotl · 2021-05-05T17:34:09Z

This is cool 😎 We are doing something similar in Python.

anuraaga

I'm ok with this as-is, but do have a strongish preference to collecting the docs into one if not in this PR then a follow up

anuraaga · 2021-05-06T03:54:32Z

api/all/docs/api_diff_1.1.0_vs_1.0.0.txt

@@ -0,0 +1,2 @@
+Comparing source compatibility of  against 


I think I'd still be find with diff vs current (docs/current/) to be aggregated, and we get to see all the new API in the next version very easily.

anuraaga · 2021-05-06T03:54:59Z

build.gradle.kts

+/**
+ * Locate the project's artifact of a particular version.
+ */
+fun Project.findArtifact(version: String) : File {


Fun with kotlin? :P

anuraaga · 2021-05-06T03:56:30Z

api/all/docs/api_diff_1.1.0_vs_1.0.0.txt

@@ -0,0 +1,2 @@
+Comparing source compatibility of  against 


Both of my suggestions should have been docs/apidiffs by the way :)

anuraaga · 2021-05-06T03:57:41Z

api/all/docs/api_diff_1.1.0_vs_1.0.0.txt

@@ -0,0 +1,2 @@
+Comparing source compatibility of  against 


I guess in the meantime it wouldn't be that hard for us to overwrite the file with a line that has an appropriate version if we want to. But no big deal

anuraaga · 2021-05-06T03:58:57Z

build.gradle.kts

+                    dependsOn("jar")
+                    // the japicmp "old" version is either the user-specified one, or the latest release.
+                    val userRequestedBase = project.properties["apiBaseVersion"] as String?
+                    val baselineVersion: String = userRequestedBase ?: latestReleasedVersion


I've never seen this lazy pattern in any other language so far - that's pretty sweet

jkwatson requested review from anuraaga, arminru, bogdandrutu, carlosalberto, Oberon00, pavolloffay, thisthat and tylerbenson as code owners April 27, 2021 20:46

jkwatson commented Apr 27, 2021

View reviewed changes

jkwatson force-pushed the generate_api_diffs branch from 826b7ed to 891b1ad Compare April 27, 2021 21:46

anuraaga reviewed Apr 27, 2021

View reviewed changes

build.gradle.kts Outdated Show resolved Hide resolved

build.gradle.kts Outdated Show resolved Hide resolved

build.gradle.kts Outdated Show resolved Hide resolved

anuraaga reviewed Apr 28, 2021

View reviewed changes

ghost approved these changes Apr 29, 2021

View reviewed changes

anuraaga approved these changes May 6, 2021

View reviewed changes

jkwatson force-pushed the generate_api_diffs branch 2 times, most recently from 11eb235 to 8901269 Compare May 6, 2021 20:10

jkwatson added 10 commits May 6, 2021 13:55

Add a task to compare public APIs for every stable module.

c8a7890

add a few comments and make the build task depend on the japicmp task

cf6ebee

update the api diffs after rebasing.

76a2df1

task must depend on jar

75f3031

a few script tweaks based on feedback

400e4e1

small tweak to make it a little more kotlin-idiomatic

93663e4

refactoring to make the code more clear

af69414

some more refactoring.

d8d5b64

make the latest released version a lazy val of the build script

bbc4d51

have the task depend on check, rather than build

7fa8f30

jkwatson added 5 commits May 6, 2021 13:55

put all the API diffs into a central directory

5f2d723

run at --info to diagnose CI issues

ef5def0

try an older java 11 version

85fdca7

try another combo

75a3c54

remove the info logging

a741131

jkwatson force-pushed the generate_api_diffs branch from 8901269 to a741131 Compare May 6, 2021 21:12

jkwatson merged commit bd6cfd8 into open-telemetry:main May 7, 2021

jkwatson deleted the generate_api_diffs branch May 7, 2021 00:50

jack-berg mentioned this pull request Aug 3, 2021

OTLP HTTP trace exporter #3418

Merged

trask mentioned this pull request Sep 13, 2021

Track api diff reports for soonish-to-be-stable artifacts open-telemetry/opentelemetry-java-instrumentation#4116

Closed

This was referenced Dec 19, 2021

Temurin JDK #4011

Merged

use Eclipse Temurin JDK docker image #4012

Merged

kbendick mentioned this pull request Apr 26, 2022

API / binary compatibility checking tool via revapi gradle plugin apache/iceberg#4638

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a task to compare public APIs for every stable module. #3183

Add a task to compare public APIs for every stable module. #3183

jkwatson commented Apr 27, 2021 •

edited

Loading

jkwatson Apr 27, 2021

ghost Apr 29, 2021

codecov bot commented Apr 27, 2021 •

edited

Loading

jkwatson Apr 27, 2021

anuraaga May 6, 2021

jkwatson May 6, 2021

jkwatson commented Apr 27, 2021

anuraaga Apr 28, 2021

jkwatson Apr 28, 2021

anuraaga May 6, 2021

anuraaga May 6, 2021

jkwatson May 6, 2021

jkwatson May 6, 2021

ghost left a comment

ghost Apr 29, 2021

ghost Apr 29, 2021

ocelotl commented May 5, 2021

anuraaga left a comment

anuraaga May 6, 2021

anuraaga May 6, 2021

anuraaga May 6, 2021

anuraaga May 6, 2021

anuraaga May 6, 2021

Add a task to compare public APIs for every stable module. #3183

Add a task to compare public APIs for every stable module. #3183

Conversation

jkwatson commented Apr 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 27, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkwatson commented Apr 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocelotl commented May 5, 2021

anuraaga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkwatson commented Apr 27, 2021 •

edited

Loading

codecov bot commented Apr 27, 2021 •

edited

Loading