RUM-11784: Update RUM Schema to include app launch vitals#2937
Conversation
|
🎯 Code Coverage 🔗 Commit SHA: a22b805 | Docs | Was this helpful? Give us feedback! |
| data class Class( | ||
| val name: String, | ||
| val properties: List<TypeProperty>, | ||
| val required: Set<String>, |
There was a problem hiding this comment.
The idea is the following.
We store the information about the required fields here instead of storing val optional: Boolean in TypeDefinition. Because if there is no TypeDefinition corresponding to the key in required block, we lose the information that this field is required. And if this key actually occurs in another item of allOf we don't treat it as required after merging (this is what I want to fix).
| "$ref": "subfolder/external_description_proxy.json#/definitions/contact" | ||
| } | ||
| }, | ||
| "required": [ |
There was a problem hiding this comment.
These item and destination don't actually exists in this schema. Before my fix the test passed, because the generator only looked at this required block while iterating through the properties (to determine the val optional: Boolean field).
| "description": "User provided context", | ||
| "additionalProperties": true | ||
| }, | ||
| "stream": { |
There was a problem hiding this comment.
This isn't related to vitals, some other new stuff.
There was a problem hiding this comment.
let's maybe just remove this if not needed for now?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2937 +/- ##
===========================================
- Coverage 70.87% 70.77% -0.10%
===========================================
Files 829 829
Lines 30384 30383 -1
Branches 5183 5183
===========================================
- Hits 21534 21503 -31
- Misses 7378 7413 +35
+ Partials 1472 1467 -5
🚀 New features to boost your workflow:
|
0xnm
left a comment
0xnm
left a comment
There was a problem hiding this comment.
lgtm, but I think we need to remove properties/files from this PR which we don't use yet (and maybe we never will).
| "description": "User provided context", | ||
| "additionalProperties": true | ||
| }, | ||
| "stream": { |
There was a problem hiding this comment.
let's maybe just remove this if not needed for now?
a0210c0 to
a22b805
Compare
| fun fromJson(kotlin.String): Configuration | ||
| fun fromJsonObject(com.google.gson.JsonObject): Configuration | ||
| data class DdVital | ||
| data class Vital1 |
There was a problem hiding this comment.
not the best naming, but I guess we can live with that
| fun fromJsonObject(com.google.gson.JsonObject): Device | ||
| data class Dd | ||
| constructor(DdSession? = null, Configuration? = null, kotlin.String? = null, kotlin.String? = null, DdVital? = null) | ||
| constructor(DdSession? = null, Configuration? = null, kotlin.String? = null, kotlin.String? = null, Vital1? = null, Profiling? = null) |
There was a problem hiding this comment.
just a sanity check: property name is still vital?
4 participants
What does this PR do?
This PR pulls the most recent RUM events schema and updates the code to use it.
The goal is to use the new application launch vital introduced here. But it will be done in future PRs. I plan to merge this PR to develop.
The current PR:
key_1should be non-null in the resultingdata class. See other comments in the PR for further explanation. This change is needed to properly support this change. Otherwiseviewbecomes optional for example inErrorEventwhich is not what we intended to do.Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)