New flag to scrub IP addresses by maxgolov · Pull Request #1161 · microsoft/cpp_client_telemetry

maxgolov · 2023-06-06T05:09:40Z

Implement IP address scrub.

Scrub IP addresses by default.

lalitb · 2023-06-07T21:50:44Z

MacOS CI tests failure can be ignored for now.

lalitb · 2023-09-12T20:15:18Z

@maxgolov Do you have plan to mark it for review, or else close if this is not valid ?

maxgolov · 2024-01-18T20:58:08Z

@lalitb - we need it, but we identified an extra scenario that's not covered by the patch. I'll do another iteration on it.

The initial change set RECORD_FLAGS_EVENTTAG_SCRUB_IP unconditionally for every event, forcing IP scrubbing on all SDK consumers in direct-upload mode -- a breaking change for apps that need client IP (e.g. geo-location). Make scrubbing the default but opt-out: the decorator sets the SCRUB_IP record flag unless CFG_BOOL_ENABLE_IP_SCRUBBING is explicitly set to false. record.flags is forwarded on the cross-platform/direct-upload path, so this redacts client IP at the collector without relying on ext.metadata privacy tags. - ILogConfiguration.hpp: add CFG_BOOL_ENABLE_IP_SCRUBBING config key - EventPropertiesDecorator.hpp: gate SCRUB_IP flag behind the config - EventPropertiesDecoratorTests.cpp: add default-on, opt-out, explicit-enable tests with a per-instance ConfigurableLogManager helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…bbing - Statistics.cpp: SDK statistics/metastats events bypass EventPropertiesDecorator, so apply the same collector-side client-IP scrub (gated by CFG_BOOL_ENABLE_IP_SCRUBBING, on by default) to those records too. Closes the gap identified in PR review. - LogConfigurationKey.java + ODWLogConfiguration.{h,mm}: expose CFG_BOOL_ENABLE_IP_SCRUBBING ('enableIpScrubbing') to the Android (Java) and Apple (Obj-C) wrappers for API parity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR introduces a new configuration flag to request collector-side scrubbing (obfuscation) of the client IP address, and ensures the flag is applied consistently both for normal telemetry events and internal stats events.

Changes:

Added CFG_BOOL_ENABLE_IP_SCRUBBING / enableIpScrubbing configuration key across C++ public config, Objective-C wrapper keys, and Android wrapper keys.
Set a new on-wire record flag (RECORD_FLAGS_EVENTTAG_SCRUB_IP) by default (unless explicitly opted out via config) in EventPropertiesDecorator.
Ensured stats/meta-stats records (which bypass EventPropertiesDecorator) also set the same scrub flag, and added unit tests covering default/opt-out/explicit-enable behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
wrappers/obj-c/ODWLogConfiguration.mm	Adds the Objective-C wrapper string constant for `enableIpScrubbing`.
wrappers/obj-c/ODWLogConfiguration.h	Exposes the Objective-C wrapper constant declaration for `enableIpScrubbing`.
tests/unittests/EventPropertiesDecoratorTests.cpp	Adds isolated configuration plumbing for tests and verifies default/opt-out/explicit-enable behavior for the scrub flag.
lib/stats/Statistics.cpp	Applies the scrub-IP record flag to stats records (which bypass the main decorator path).
lib/include/public/ILogConfiguration.hpp	Adds the public C++ configuration key constant and documentation for `enableIpScrubbing`.
lib/decorators/EventPropertiesDecorator.hpp	Introduces `RECORD_FLAGS_EVENTTAG_SCRUB_IP` and sets it by default unless config opts out.
lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java	Adds the Android wrapper enum entry for `enableIpScrubbing`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The doc implied the setting only applies in direct-upload mode, but the scrub flag is set for all events and modes. Clarify that the flag is honored by the OneCollector direct-upload path while UTC mode applies its own client-privacy handling. No behavior change -- the flag is intentionally mode-agnostic and is ignored by the UTC pipeline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

…fig docs - EventPropertiesDecorator.hpp: include ILogManager.hpp so the header is self-contained for the ILogManager (m_owner) and ILogConfiguration types / CFG_BOOL_ENABLE_IP_SCRUBBING used inline, instead of relying on the includer. - Clarify CFG_BOOL_ENABLE_IP_SCRUBBING docs (C++ / Java / Obj-C): scrubbing is applied unless explicitly set to false (on by default) and the key is not present in the default configuration, so GetDefaultConfiguration() does not surface it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

…hared header Move the RECORD_FLAGS_EVENTTAG_* on-wire bits out of EventPropertiesDecorator.hpp into a dedicated decorators/RecordFlagConstants.hpp, exposed as static constexpr std::int64_t in the MAT namespace (no longer #define macros). This lets the stats pipeline reference RECORD_FLAGS_EVENTTAG_SCRUB_IP via the small shared header instead of pulling in the full decorator header, and avoids macro pollution. - New: lib/decorators/RecordFlagConstants.hpp - EventPropertiesDecorator.hpp: include the shared header; drop the macros - Statistics.cpp: include the shared header instead of EventPropertiesDecorator.hpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

… m_room/element) Defensive follow-up to the #1227 family of Android Room crashes ("java_object == null in call to GetObjectClass" under GetAndReserveRecords). #1417 fixed the stale-local-ref root cause but left two null paths unguarded, which can still hard-abort the host process: * m_room is null when the Room DB failed to open or was torn down (the destructor already guards with `if (s_vm && m_room)`, but ~10 other methods dereferenced it unconditionally). Add `if (!m_room) return <fail>;` guards to DeleteRecords(x2), GetAndReserveRecords, ReleaseRecords, StoreRecords, DeleteSetting, StoreSetting, GetSizeInternal, GetRecordCount, ResizeDbInternal, and GetRecords. GetSetting already had this guard. * a null element in the getAndReserve/releaseRecords result array (observed to be androidx.room-version sensitive) was passed to GetObjectClass. Guard both loops: GetAndReserveRecords pops the frame and stops (the existing index < limit path releases the rest for retry); ReleaseRecords skips it. Turns a process-killing JNI abort into graceful degradation. No functional change on the healthy path. Compiles on Android only (JNI/Room) -- validated by CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

…follow-up) cpp-start-android.md covered Room setup but said nothing about the Room version. The GetAndReserveRecords native crash (#1227, Room-version-sensitive) and the null-guard hardening in this PR make this worth documenting: the maesdk AAR brings androidx.room transitively (pinned in maesdk/build.gradle, currently 2.8.4); since Gradle resolves one Room version app-wide, consumers should not force a version below what the SDK is built against and should prefer aligning on the bundled (or a compatible newer) version. Points at build.gradle as the source of truth so the doc doesn't drift on future bumps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

…ull element Address the latest Copilot review (6 comments) on lib/offline/OfflineStorage_Room.cpp. ConnectedEnv null-env guards (5): DeleteByToken, ReleaseRecords, DeleteSetting, StoreSetting and GetRecords created ConnectedEnv env(s_vm) and dereferenced env->... behind only an if(!m_room) guard. ConnectedEnv::operator! can report a null JNIEnv (null s_vm / thread-attach failure), and sibling methods already guard with if(!env); added the matching early return (void/false/records) so a null env no longer crashes. releaseUnconsumed on null element: the null-array-element path broke out and fell through to releaseUnconsumed(selected, index). The Java StorageRecordDao.releaseUnconsumed (pre-existing since 2020, commit c81d46a) reads selected[0..unconsumed-1] ignoring the offset, so on the null path it could index the null element and throw, or release the wrong rows. A sawNullElement flag now skips releaseUnconsumed on that path; the reserved records expire and are retried (no data loss), and the normal end-early path is unchanged. Validated: NDK aarch64-linux-android23 -fsyntax-only clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

madhuriguptamicrosoft · 2026-06-17T09:59:49Z

I've validated the changes in the mobile app. The IP is currently being scrubbed.

Update EventPropertiesDecorator.hpp

e83c8ab

Scrub IP addresses by default.

maxgolov requested review from ThomsonTan, bliptec, kindbe, lalitb, mkoscumb, reyang and sid-dahiya as code owners June 6, 2023 05:09

maxgolov marked this pull request as draft June 6, 2023 05:10

lalitb approved these changes Jun 7, 2023

View reviewed changes

Merge branch 'main' into maxgolov/scrub_ip_by_default

d1be28c

maxgolov and others added 5 commits January 18, 2024 12:58

Merge branch 'main' into maxgolov/scrub_ip_by_default

ca39292

Merge branch 'main' into maxgolov/scrub_ip_by_default

79c544c

Merge branch 'main' into maxgolov/scrub_ip_by_default

e2bf38a

Merge branch 'main' into maxgolov/scrub_ip_by_default

9854cc7

bmehta001 self-assigned this Jun 10, 2026

bmehta001 requested review from Copilot and removed request for bliptec and kindbe June 11, 2026 00:34

Copilot started reviewing on behalf of bmehta001 June 11, 2026 00:34 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Comment thread lib/include/public/ILogConfiguration.hpp

bmehta001 marked this pull request as ready for review June 11, 2026 01:40

bmehta001 requested a review from a team as a code owner June 11, 2026 01:40

bmehta001 requested a review from Copilot June 11, 2026 01:40

Copilot started reviewing on behalf of bmehta001 June 11, 2026 01:40 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Comment thread lib/decorators/EventPropertiesDecorator.hpp Outdated

Comment thread lib/include/public/ILogConfiguration.hpp

bmehta001 requested a review from Copilot June 11, 2026 01:55

Copilot started reviewing on behalf of bmehta001 June 11, 2026 01:55 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Comment thread lib/stats/Statistics.cpp

Comment thread lib/decorators/EventPropertiesDecorator.hpp Outdated

bmehta001 requested a review from Copilot June 11, 2026 02:06

Copilot started reviewing on behalf of bmehta001 June 11, 2026 02:07 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Merge branch 'main' into maxgolov/scrub_ip_by_default

3bea614

bmehta001 mentioned this pull request Jun 11, 2026

Forward ext.metadata (privTags) on the cross-platform serialization path #1472

Closed

bmehta001 requested a review from Copilot June 16, 2026 08:23

Copilot started reviewing on behalf of bmehta001 June 16, 2026 08:24 View session

Copilot AI reviewed Jun 16, 2026

View reviewed changes

bmehta001 requested a review from Copilot June 16, 2026 17:16

Copilot started reviewing on behalf of bmehta001 June 16, 2026 17:17 View session

Copilot AI reviewed Jun 16, 2026

View reviewed changes

bmehta001 and others added 2 commits June 16, 2026 19:43

Merge branch 'main' into maxgolov/scrub_ip_by_default

c9e3cc4

bmehta001 requested a review from Copilot June 17, 2026 00:58

Copilot started reviewing on behalf of bmehta001 June 17, 2026 00:58 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

Conversation

maxgolov commented Jun 6, 2023

Uh oh!

lalitb commented Jun 7, 2023

Uh oh!

lalitb commented Sep 12, 2023

Uh oh!

maxgolov commented Jan 18, 2024

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

madhuriguptamicrosoft commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants