Skip to content

Reorganise gesture instrumentation into separate modules - #1841

Open
DavidGrath wants to merge 2 commits into
open-telemetry:mainfrom
DavidGrath:issue-1576-reorganize-gestures
Open

Reorganise gesture instrumentation into separate modules#1841
DavidGrath wants to merge 2 commits into
open-telemetry:mainfrom
DavidGrath:issue-1576-reorganize-gestures

Conversation

@DavidGrath

@DavidGrath DavidGrath commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

References #1576

To solve the issue of shared code being exposed, I created a new annotation similar to Incubating.
Also left 2 TODO's to be discussed.

I tried working on the Scale gesture as well, but it became so involved that I decided it should just be a follow-up: I added another module for the sake of working with Robolectric more effectively, amongst other things

Copilot AI review requested due to automatic review settings June 25, 2026 07:10
@DavidGrath
DavidGrath requested a review from a team as a code owner June 25, 2026 07:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR reorganizes gesture-related view instrumentation by extracting shared gesture/view utilities into a new instrumentation:common-api area and introducing a dedicated instrumentation:view-scroll module, while updating existing view-click code/tests and shared test utilities accordingly.

Changes:

  • Added a new view-scroll instrumentation module (implementation, tests, docs, and API surface).
  • Moved previously view-click-local gesture/view utility code into instrumentation:common-api behind a new opt-in annotation.
  • Centralized reusable MotionEvent/test helpers into test-common (and updated instrumentation tests to import them).

Merge Tier (per repo guidelines): Tier 4 (public API surface changes / new .api files added/updated)

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 13 comments.

Show a summary per file
File Description
test-common/src/test/kotlin/io/opentelemetry/android/test/common/ViewTestUtilsTest.kt Adds package + renames test class to align with new ViewTestUtils packaging.
test-common/src/main/kotlin/io/opentelemetry/android/test/common/ViewTestUtils.kt Moves utilities into the io.opentelemetry.android.test.common package and tweaks formatting/imports.
test-common/build.gradle.kts Adds mocking bundle dependency to support test-common implementations.
instrumentation/view-scroll/src/test/kotlin/io/opentelemetry/android/instrumentation/view/scroll/ViewScrollInstrumentationTest.kt New/ported tests for scroll/fling event emission and attribute assertions.
instrumentation/view-scroll/src/main/kotlin/io/opentelemetry/android/instrumentation/view/scroll/WindowCallbackWrapper.kt Callback wrapper to intercept touch events for scroll instrumentation.
instrumentation/view-scroll/src/main/kotlin/io/opentelemetry/android/instrumentation/view/scroll/ViewScrollInstrumentation.kt New AutoService instrumentation entry point for view scroll.
instrumentation/view-scroll/src/main/kotlin/io/opentelemetry/android/instrumentation/view/scroll/ViewScrollEventGenerator.kt New event generator that turns gesture detector callbacks into OTel log events.
instrumentation/view-scroll/src/main/kotlin/io/opentelemetry/android/instrumentation/view/scroll/ViewScrollActivityCallback.kt Lifecycle callback to start/stop window tracking.
instrumentation/view-scroll/README.md Documents emitted events and attributes for the new module.
instrumentation/view-scroll/build.gradle.kts Declares the new published module and its dependencies.
instrumentation/view-scroll/api/view-scroll.api Adds API dump for the new ViewScrollInstrumentation public surface.
instrumentation/view-click/src/test/kotlin/io/opentelemetry/android/instrumentation/view/click/ViewLongPressInstrumentationTest.kt Updates imports/constants to use new shared test helpers + shared internal constants.
instrumentation/view-click/src/test/kotlin/io/opentelemetry/android/instrumentation/view/click/ViewClickInstrumentationTest.kt Updates imports/constants to use new shared test helpers + shared internal constants.
instrumentation/view-click/src/main/kotlin/io/opentelemetry/android/instrumentation/view/click/WindowCallbackWrapper.kt Marks wrapper with the new internal opt-in annotation.
instrumentation/view-click/src/main/kotlin/io/opentelemetry/android/instrumentation/view/click/ViewClickInstrumentation.kt Switches to the new internal opt-in annotation + common-api dependency.
instrumentation/view-click/src/main/kotlin/io/opentelemetry/android/instrumentation/view/click/ViewClickEventGenerator.kt Replaces in-file view traversal/helpers with common-api equivalents; removes scroll/fling logic from click generator.
instrumentation/view-click/src/main/kotlin/io/opentelemetry/android/instrumentation/view/click/ViewClickActivityCallback.kt Marks lifecycle callback with the new internal opt-in annotation.
instrumentation/view-click/src/main/kotlin/io/opentelemetry/android/instrumentation/view/click/internal/ViewUtils.kt Deletes old, module-local constants/helpers now moved to common-api.
instrumentation/view-click/build.gradle.kts Adds dependency on instrumentation:common-api.
instrumentation/common-api/src/main/java/io/opentelemetry/android/instrumentation/internal/ViewUtils.kt New shared constants, gesture model, and view hit-testing helpers (currently public ABI).
instrumentation/common-api/src/main/java/io/opentelemetry/android/instrumentation/internal/InternalViewApi.kt New opt-in annotation intended to restrict cross-module “internal view” APIs.
instrumentation/common-api/api/common-api.api Updates API dump to include newly added symbols from the internal view utilities.
Comments suppressed due to low confidence (6)

test-common/src/main/kotlin/io/opentelemetry/android/test/common/ViewTestUtils.kt:240

  • Minor ktlint/Spotless formatting: add a space after if.
    test-common/src/main/kotlin/io/opentelemetry/android/test/common/ViewTestUtils.kt:60
  • The parameter wrapping/indentation for this function signature doesn’t match the surrounding Kotlin formatting used elsewhere in the repo (and may trip Spotless/ktlint). Consider re-wrapping using the standard multiline parameter style.
    instrumentation/view-click/src/test/kotlin/io/opentelemetry/android/instrumentation/view/click/ViewLongPressInstrumentationTest.kt:52
  • Use @OptIn(InternalViewApi::class) to opt-in to the internal view APIs used in this test, rather than annotating the test class itself with @InternalViewApi. Also remove the JUnit5 @ExtendWith since it won’t be applied under AndroidJUnit4.
@InternalViewApi
@OptIn(IncubatingApi::class)
@RunWith(AndroidJUnit4::class)
@ExtendWith(MockKExtension::class)
class ViewLongPressInstrumentationTest {

instrumentation/view-scroll/src/test/kotlin/io/opentelemetry/android/instrumentation/view/scroll/ViewScrollInstrumentationTest.kt:24

  • This test uses the JUnit4 AndroidJUnit4 runner, so the JUnit5 MockKExtension won’t be applied. Removing the mockk-junit5 import avoids misleading annotations/dependencies.
    instrumentation/view-scroll/src/test/kotlin/io/opentelemetry/android/instrumentation/view/scroll/ViewScrollInstrumentationTest.kt:54
  • Since this is a JUnit4 test (@RunWith(AndroidJUnit4::class)), remove the JUnit5 ExtendWith import/annotation to keep the test configuration consistent.
    instrumentation/view-scroll/src/test/kotlin/io/opentelemetry/android/instrumentation/view/scroll/ViewScrollInstrumentationTest.kt:59
  • Use @OptIn(InternalViewApi::class) to opt-in within the test, rather than marking the test class itself as an internal API via @InternalViewApi. Also drop the JUnit5 @ExtendWith since it won’t run under AndroidJUnit4.

Comment on lines 20 to 24
@@ -27,12 +24,17 @@
import io.mockk.slot
Comment on lines 50 to 53
import org.junit.Before
import org.junit.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.runner.RunWith
Comment on lines 18 to 22
import io.mockk.every
import io.mockk.impl.annotations.MockK
import io.mockk.junit5.MockKExtension
import io.mockk.mockk
import io.mockk.slot
Comment on lines 43 to 46
import org.junit.Before
import org.junit.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.runner.RunWith
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.30081% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.39%. Comparing base (f07f652) to head (bad14a5).
⚠️ Report is 63 commits behind head on main.

Files with missing lines Patch % Lines
...y/android/instrumentation/view/common/ViewUtils.kt 69.44% 6 Missing and 5 partials ⚠️
...umentation/view/scroll/ViewScrollEventGenerator.kt 87.87% 4 Missing and 4 partials ⚠️
...trumentation/view/click/ViewClickEventGenerator.kt 60.00% 0 Missing and 2 partials ⚠️
...mentation/view/common/ViewClickActivityCallback.kt 75.00% 1 Missing ⚠️
...mentation/view/scroll/ViewScrollInstrumentation.kt 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1841      +/-   ##
==========================================
+ Coverage   65.34%   65.39%   +0.05%     
==========================================
  Files         169      171       +2     
  Lines        3872     3895      +23     
  Branches      420      424       +4     
==========================================
+ Hits         2530     2547      +17     
- Misses       1211     1215       +4     
- Partials      131      133       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@LikeTheSalad LikeTheSalad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you! 🙏 I like the idea in general, but I wanted to discuss more about the folder structure and where to put scoped common apis.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, this is the only file that needs to be shared across the 2 view-* modules? If that's the case, I think the common-api might be too much of a generic place for it. This is the first time we want to split a couple of instrumentations that share the same "scope", so please bear with me as we go through the process of designing how these use cases should look like.

Ideally, I think we should have a "common-view-scoped" shared module, where we put this ViewUtils type. Regarding the folder structure, what do you think about the following:

instrumentation/
├─ view/
│  ├─ click/
│  ├─ scroll/
│  ├─ common/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is a cool idea, but I'm not sure it's needed to go this far, given that the package for ViewUtils already contains internal; it should mean to any user that they shouldn't use that type because it can change at any time (we can add an explanatory comment like this one, for example).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I actually like this idea
My initial impression was that the module would be broken down into top-level instrumentations, but putting them under a common module as suggested above actually works better because now I can use internal exclusively. I'll work with that

@DavidGrath
DavidGrath force-pushed the issue-1576-reorganize-gestures branch from 7e686d4 to 70af8fd Compare June 30, 2026 08:13
@breedx-splk

Copy link
Copy Markdown
Contributor

Sorry about the merge conflicts @DavidGrath .... this got caught up in the re-linting of the entire kotlin codebase.

@DavidGrath
DavidGrath force-pushed the issue-1576-reorganize-gestures branch from b9a074f to 6a79c4f Compare July 21, 2026 05:22
@breedx-splk

Copy link
Copy Markdown
Contributor

@DavidGrath there are a number of open/unanswered copilot review comments. Please have a look at those, respond or close. That can help move this forward (we don't want a bunch of dangling threads).

To me, it also looks like both view-click and view-scroll depend on view-common, but view-common doesn't include the otel.publish-conventions, so how is that supposed to work once those hit maven?

@fractalwrench fractalwrench added the needs author feedback Waiting for additional feedback from the author label Aug 5, 2026
@@ -0,0 +1,22 @@
plugins {
id("otel.android-library-conventions")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This probably also needs the publish conventions, so that the dependent modules can consume it from maven/sonatype after they are published.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs author feedback Waiting for additional feedback from the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants