Add typed properties for analytics events #823
Conversation
This is a work in progress, it breaks One Collector and tests.
sdk/appcenter-analytics/src/main/java/com/microsoft/appcenter/analytics/Analytics.java
Outdated
Show resolved
Hide resolved
…' into feature/event-typed-properties-app
guperrot
changed the title
| * <ul> | ||
| * <li>The event name needs to match the [a-zA-Z0-9]((\.(?!(\.|$)))|[_a-zA-Z0-9]){3,99} regular expression.</li> | ||
| * <li>The baseData and baseDataType properties are reserved and thus discarded.</li> | ||
| * <li>The full event size when encoded in JSON cannot be larger than 1.9MB.</li> |
There was a problem hiding this comment.
The full event size when encoded in JSON to JSON string cannot be larger than 1.9MB.
| * For OneCollector: | ||
| * <ul> | ||
| * <li>The event name needs to match the [a-zA-Z0-9]((\.(?!(\.|$)))|[_a-zA-Z0-9]){3,99} regular expression.</li> | ||
| * <li>The baseData and baseDataType properties are reserved and thus discarded.</li> |
There was a problem hiding this comment.
Can we use "baseData" and "baseDataType"? We need to wrap reserved keyword in quotes.
...ectDependency/java/com/microsoft/appcenter/sasquatch/activities/EventPropertiesActivity.java
Show resolved
Hide resolved
| if (!super.equals(o)) { | ||
| return false; | ||
| } | ||
| if (this == o) return true; |
There was a problem hiding this comment.
I really don't like one-line IF statements. I know it's just the equals method but this makes my skin crawl in a bad way.
There was a problem hiding this comment.
This is generated code ;)
For equals/haschode, we just use the IDE to generate or regenerate when we add or remove a field.
| if (key.length() > MAX_PROPERTY_ITEM_LENGTH) { | ||
| message = String.format("Typed property '%s' : property key length cannot be longer than %s characters. Property key will be truncated.", key, MAX_PROPERTY_ITEM_LENGTH); | ||
| AppCenterLog.warn(LOG_TAG, message); | ||
| property.setName(key.substring(0, MAX_PROPERTY_ITEM_LENGTH)); |
There was a problem hiding this comment.
This should change a copy of the property, not the property itself. Even though they are moved to another array, that array contains references to properties that will be used in other logs as well. (same comment on line 189)
…rrent-exception Avoid concurrent modification exception when doing property traversal
| * <p> | ||
| * Additional validation rules apply depending on the configured secret. | ||
| * <p> | ||
| * For AppCenter: |
There was a problem hiding this comment.
nit: AppCenter -> App Center. (Same with OneCollector -> One Collector)
| * <p> | ||
| * For AppCenter: | ||
| * <ul> | ||
| * <li>the event name cannot be longer than 256 and is truncated otherwise.</li> |
There was a problem hiding this comment.
Nit: these list items should be capitalized (like the ones for OC)
| * <p> | ||
| * For AppCenter: | ||
| * <ul> | ||
| * <li>the event name cannot be longer than 256 and is truncated otherwise.</li> |
| * <p> | ||
| * Additional validation rules apply depending on the configured secret. | ||
| * <p> | ||
| * For AppCenter: |
| * <li>the number of properties per event is limited to 20 (truncated).</li> | ||
| * </ul> | ||
| * <p> | ||
| * For OneCollector: |
| * <p> | ||
| * Additional validation rules apply depending on the configured secret. | ||
| * <p> | ||
| * For AppCenter, the name cannot be longer than 256 and is truncated otherwise. |
|
Ingestion changes are in prod. |
Feature needs ingestion to be deployed to prod before merge.