Skip to content

Log analytics event when Entity create/update fails to apply#7238

Open
grzesiek2010 wants to merge 7 commits into
getodk:masterfrom
grzesiek2010:COLLECT-7230
Open

Log analytics event when Entity create/update fails to apply#7238
grzesiek2010 wants to merge 7 commits into
getodk:masterfrom
grzesiek2010:COLLECT-7230

Conversation

@grzesiek2010
Copy link
Copy Markdown
Member

@grzesiek2010 grzesiek2010 commented May 20, 2026

Closes #7230

Why is this the best possible solution? Were any other approaches considered?

I’ve added the new analytics we discussed earlier. With these analytics in place, I’m wondering whether we still need to log invalid entities to a file that is only available in debug builds (introduced in d3cb260).

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

It doesn't require testing.

Do we need any specific form for testing your changes? If so, please attach one.

No.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

Before submitting this PR, please make sure you have:

  • added or modified tests for any new or changed behavior
  • run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
  • added a comment above any new strings describing it for translators
  • added any new strings with date formatting to DateFormatsTest
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

@grzesiek2010 grzesiek2010 marked this pull request as ready for review May 20, 2026 21:58
@grzesiek2010 grzesiek2010 requested a review from seadowg May 20, 2026 21:58
Copy link
Copy Markdown
Member

@seadowg seadowg left a comment

Choose a reason for hiding this comment

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

With these analytics in place, I’m wondering whether we still need to log invalid entities to a file that is only available in debug builds

Could the DebugLogger implementation also log analytics? I think that'd be useful to limit noise in the implementations themselves.

Comment thread entities/src/main/java/org/odk/collect/entities/analytics/AnalyticsEvents.kt Outdated
@grzesiek2010
Copy link
Copy Markdown
Member Author

grzesiek2010 commented May 28, 2026

Could the DebugLogger implementation also log analytics?

I'm hesitant about this because DebugLogger is active in debug builds only, while we need these analytics in production. Combining them would be misleading. Are you even using this logger?

@grzesiek2010 grzesiek2010 requested a review from seadowg May 28, 2026 12:58
@seadowg
Copy link
Copy Markdown
Member

seadowg commented May 29, 2026

I'm hesitant about this because DebugLogger is active in debug builds only, while we need these analytics in production. Combining them would be misleading.

Good point. We could have the logger always active and only log to a file in debug?

Are you even using this logger?

We were initially planning to expand it (and access to it) over time and possible present UI for it so that form designers can use it or the log can be sent to us to help resolve problems.

@grzesiek2010
Copy link
Copy Markdown
Member Author

We could have the logger always active and only log to a file in debug?

We can do that.

I've moved the ID validation to LocalEntityUseCases, see d060b27.

I remember you weren't a fan of this change when it was introduced some time ago, but it makes more sense now, especially after introducing the UPSERT action. Previously, the collection of invalid entities only included entities with blank or invalid IDs, while other validation cases were still being checked in LocalEntityUseCases. Because of that, it didn't seem right to maintain a collection called "invalid entities" when it actually represented only a subset of all invalid entities. We later use this information only for logging to DebugLogger or analytics anyway, so keeping all validation checks in one place feels more consistent.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Log analytics event when Entity create/update fails to apply

3 participants