[Android] Expose IDataViewer as public Java API by KartikDhawaniya · Pull Request #1534 · microsoft/cpp_client_telemetry

KartikDhawaniya · 2026-09-11T21:48:43Z

Summary

Expose the native 1DS IDataViewer extension point through the Android Java/JNI API so Android consumers can register a product-owned viewer without changing the bundled DefaultDataViewer implementation.

This enables consumers such as Teams Android to keep broad Network Security Configuration cleartext traffic disabled while owning any product-specific diagnostic transport outside the SDK.

Changes

Add the public Android IDataViewer callback contract.
Add registerDataViewer and unregisterDataViewer to ILogManager and LogManagerProvider.LogManagerImpl.
Add a JNI-backed C++ IDataViewer proxy that:
- retains the Java implementation with a global reference;
- attaches native SDK worker threads to the JVM when required;
- forwards encoded packet payloads as byte[];
- caches a stable viewer name;
- isolates and clears Java callback exceptions; and
- releases JNI references deterministically.
Retain registered proxies per native LogManager and clean them up on unregister/close.
Reject invalid, empty, duplicate, or unknown viewer registrations with explicit failure results.
Add consumer R8/ProGuard rules for reverse-JNI callbacks.
Add Android instrumentation coverage for registration, duplicate rejection, callback dispatch, callback exception isolation, and unregistration.

Scope

This is an additive Android API bridge over the existing native extension point. It does not change:

production telemetry upload or routing;
endpoint selection or authentication;
the existing DefaultDataViewer APIs; or
product-specific transport, endpoint validation, queueing, retry, or feature-gating policy.

Validation

:maesdk:assembleDebug
:app:compileDebugAndroidTestJavaWithJavac
Native Android builds for arm64-v8a, armeabi-v7a, x86, and x86_64

The instrumentation test was compiled but not executed because no Android device was connected.

KartikDhawaniya · 2026-09-11T21:53:28Z

@microsoft-github-policy-service agree company="Microsoft"

Copilot

🟡 Changes recommended

The public interface change breaks source compatibility, and callback lifecycle documentation and unregistration coverage need correction.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a public Android/JNI bridge for registering product-owned IDataViewer implementations.

Changes:

Adds the Java callback API and LogManager registration methods.
Implements JNI proxy lifecycle, callback forwarding, and cleanup.
Adds shrinker rules and instrumentation coverage.

File summaries

File	Description
`lib/jni/LogManager_jni.cpp`	Manages viewer registration and cleanup.
`lib/jni/JavaDataViewerProxy.hpp`	Declares the JNI viewer proxy.
`lib/jni/JavaDataViewerProxy.cpp`	Implements Java callback forwarding.
`lib/CMakeLists.txt`	Builds the new proxy source.
`lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogManagerProvider.java`	Implements Java registration APIs.
`lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/ILogManager.java`	Exposes registration publicly.
`lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/IDataViewer.java`	Defines the callback contract.
`lib/android_build/maesdk/consumer-rules.pro`	Preserves reverse-JNI callback methods.
`lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/LogManagerDDVUnitTest.java`	Tests registration and dispatch behavior.

Review details

Files reviewed: 9/9 changed files
Comments generated: 3
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+ * Callbacks can occur on an SDK worker thread and should return promptly. Implementations must not
+ * register or unregister viewers from within a callback.


Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

bmehta001 · 2026-09-18T17:27:20Z

I reviewed whether this API is necessary and proportionate. The capability is justified, but the PR should not merge unchanged.

Android currently has no Java mechanism for registering a product-owned implementation of the native IDataViewer extension point. The existing DefaultDataViewer path owns its HTTP transport and requires a cleartext http:// private-subnet endpoint, so it does not address the stated product need without broader cleartext configuration.

The JNI bridge is therefore a reasonable direction, but the existing inline findings are merge blockers:

Adding abstract methods to public ILogManager creates an avoidable source-compatibility break; use source-compatible defaults or a separate extension interface.
Reentrant close() from receiveData() can remove viewers while native dispatch is iterating the collection.
The instrumentation test must verify that callbacks actually stop after unregistering and should be executed on a device/emulator.

With those addressed, the scope is proportionate to the demonstrated gap.

Preserves source compatibility, fixes reentrant dispatch, and strengthens the unregistration test. registerDataViewer and unregisterDataViewer become default methods on ILogManager returning false. Adding abstract methods to a public interface would break every consumer-owned implementation and test double on upgrade, despite the change being additive in intent. LogManagerImpl overrides both, so the native path is unaffected. DispatchDataViewerEvent now iterates a snapshot of the viewer collection rather than the member vector. m_dataViewerMapLock is recursive, so a viewer that reenters the SDK from ReceiveData - closing the owning LogManager, which unregisters every viewer - was admitted back in and erased the vector while dispatch was still walking it, invalidating the iterator. Exposing IDataViewer to arbitrary Java implementations makes that reachable from outside the SDK, so the hazard is fixed rather than only documented. Holding shared_ptr copies also keeps each viewer alive across its own callback. The IDataViewer contract now prohibits closing the owning manager from a callback, and a unit test covers a viewer that unregisters everything from ReceiveData. The instrumentation test asserted only the native return value of unregisterDataViewer, so a bridge that dropped its bookkeeping entry but left the proxy registered in DataViewerCollection would have passed. It now drives a second dispatch after unregistering, using the still registered throwing viewer as the witness that a dispatch really occurred, and asserts the unregistered viewer's callback count does not increase. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

IDataViewer returns the endpoint by const reference, so the referent must outlive the call and must not be mutated while a caller holds it. The proxy updated a shared member under a mutex and then returned a reference to it, releasing the lock on return: two concurrent callers could read and write the same string at once, so the mutex gave no protection. Use a thread_local buffer instead, which gives each calling thread its own storage and removes the need for the lock. Behaviour is unchanged: the endpoint is still read from Java on every call, so a viewer that changes endpoints still reports the current one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

Callback dispatch can deadlock through inverted locks, and JNI allocation failures can leave pending exceptions uncleared.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 1 · 1

Open (3)

Release collection lock before invoking Java callbacks · New
Clear JNI exception when NewByteArray fails · New
The reentrancy warning also needs to prohibit closing the owning manager from a callback. close()…

Resolved since last review (2)

These are abstract methods on an existing public interface, so any consumer-owned implementation or…
This assertion checks only the native return value; it does not verify that callbacks stop after…

+        // erase from the very vector being iterated here and invalidate the iterator.
+        // Holding shared_ptr copies additionally keeps each viewer alive for the duration of
+        // its own callback, even if that callback drops the last other reference to it.
+        const auto viewers = m_dataViewerCollection;


+        }
+
+        auto packet = env->NewByteArray(static_cast<jsize>(packetData.size()));
+        if (packet == nullptr || ClearPendingException(env, "receiveData allocation"))


KartikDhawaniya requested a review from a team as a code owner September 11, 2026 21:48

bmehta001 requested a balanced review from Copilot September 12, 2026 06:38

Copilot started reviewing on behalf of bmehta001 September 12, 2026 06:39 View session

Copilot AI reviewed Sep 12, 2026

View reviewed changes

[Android] Expose IDataViewer as public Java API

866832b

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

KartikDhawaniya force-pushed the user/kdhawaniya/android-idataviewer-api branch from 103ad8e to 866832b Compare September 16, 2026 10:27

kdhawaniya and others added 3 commits September 21, 2026 16:58

Merge branch 'main' into user/kdhawaniya/android-idataviewer-api

8e97566

bmehta001 requested a balanced review from Copilot September 22, 2026 16:26

Copilot started reviewing on behalf of bmehta001 September 22, 2026 16:30 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Navigation Menu

[Android] Expose IDataViewer as public Java API - #1534

[Android] Expose IDataViewer as public Java API#1534
KartikDhawaniya wants to merge 4 commits into
microsoft:mainfrom
KartikDhawaniya:user/kdhawaniya/android-idataviewer-api

KartikDhawaniya commented Sep 11, 2026

Uh oh!

KartikDhawaniya commented Sep 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

bmehta001 commented Sep 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		* Callbacks can occur on an SDK worker thread and should return promptly. Implementations must not
		* register or unregister viewers from within a callback.

Conversation

KartikDhawaniya commented Sep 11, 2026

Summary

Changes

Scope

Validation

Uh oh!

KartikDhawaniya commented Sep 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

bmehta001 commented Sep 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants