Update RUM schema#2976
Conversation
|
🎯 Code Coverage 🔗 Commit SHA: b48ee7a | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #2976 +/- ##
===========================================
+ Coverage 70.87% 70.88% +0.01%
===========================================
Files 829 829
Lines 30383 30384 +1
Branches 5186 5186
===========================================
+ Hits 21533 21537 +4
- Misses 7370 7374 +4
+ Partials 1480 1473 -7
🚀 New features to boost your workflow:
|
| */ | ||
| fun setVitalEventMapper(eventMapper: EventMapper<VitalEvent>): Builder { | ||
| rumConfig = rumConfig.copy(vitalEventMapper = eventMapper) | ||
| fun setVitalOperationStepEventMapper(eventMapper: EventMapper<RumVitalOperationStepEvent>): Builder { |
There was a problem hiding this comment.
Important
This is a change of the public API that was already released.
Even if we don't change the name of the method, there is still a change in the function signature (because of the argument type).
It is probably ok, because feature operations are fairly recent are still under ExperimentalRumAPI (not this particular method though).
But I'm not entirely sure.
There was a problem hiding this comment.
Is it also a similar change for iOS SDK?
Maybe we can keep both methods for now: deprecate old one and make it alias for a new one. Old one will have a change of the argument type, but this is fine, because anyway it is functional interface (so no need to specify type explicitly) and new model has almost the same structure as the old one.
There was a problem hiding this comment.
Looks like they don't have a mapper for the vital events at all.
There was a problem hiding this comment.
but this is fine, because anyway it is functional interface (so no need to specify type explicitly)
import com.datadog.android.event.EventMapper
import com.datadog.android.rum.model.RumVitalOperationStepEvent
object SomeMapper : EventMapper<RumVitalOperationStepEvent> {
override fun map(event: RumVitalOperationStepEvent): RumVitalOperationStepEvent? {
TODO("Not yet implemented")
}
}
If you want to implement the mapper in a separate file, you have to. And IIUC binary compatibility is still broken.
There was a problem hiding this comment.
yes, totally. once it is not a functional SAM conversion change is not backward compatible.
There was a problem hiding this comment.
It is probably ok, because feature operations are fairly recent are still under ExperimentalRumAPI (not this particular method though).
Feature Operations are indeed marked as experimental, but setVitalEventMapper is not.
I think we can deprecate it and keep it, making it either no-op (since once we add more Vital mappers it is not clear to which one it corresponds) or forward call to setVitalOperationStepEventMapper (since it is the only Vital we have right now).
This will be more compliant with semver semantics.
There was a problem hiding this comment.
OK
- Returned
setVitalEventMapperand made itDeprecated. - Made
setVitalEventMappercallsetVitalOperationStepEventMapper. - Annotated
setVitalOperationStepEventMapperwithExperimentalRumApi. To be similar with other feature operations methods.
There was a problem hiding this comment.
The setVitalEventMapper wasn't planned as experimental as there was different types of VitalEvents and @ExperimentalRumApi was applied only for the Feature Operation. But according to the latest changes it might make sense to add this annotation for the setting mapper method as well.
fd409ec to
6043abe
Compare
6043abe to
8da3da9
Compare
5 participants
What does this PR do?
Updating RUM schema to include this change. It split vital event into 3 different events:
vital-duration-schemavital-app-launch-schemavital-operation-step-schemaThey don't have the same parent anymore. They are distinct event types.
There was also some refactoring arount
viewfield link.The changes are so huge because a lot of classes classes in the
api-surfacefiles are now in a different order for some reason.Similar PR on iOS DataDog/dd-sdk-ios#2548
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)