Add Event Emitter with Data to Android Repo #100
Conversation
instrumentation/src/main/java/io/opentelemetry/android/emitter/EventEmitterWithData.java
Outdated
Show resolved
Hide resolved
| delegate.emit(eventName, eventAttrs); | ||
| } | ||
|
|
||
| private Attributes convertRichDataObjectIntoAttributes(Map<String, Object> data) { |
There was a problem hiding this comment.
Let's double check this - Based on the data we expect in event.data for Android, are we sure we're okay to have these 4 types only - String, Long, Double, Boolean and all other types could be correctly represented by calling toString() [line 48] (As in it will not end up printing the memory address but will give actual content (value) of the attribute for any other types we expect)
There was a problem hiding this comment.
We could add other types yes, but at least when considering what this is currently planned to be used for (emitting crashes), I don't anticipate other data types unless I missed one and you can think of any? And if there are other object types, its possible that they'll have their own toString() method which will give an appropriate string representation of the object.
And if not, it will resort to the default behaviour of toString() is to print class name, then @, then unsigned hexadecimal representation of the hash code of the object. Which I think is acceptable in an edge case. As if its not an object type we expect, whatever backend may not be able to parse it anyway. I'm definitely all ears and open to an alternative approach though if you have any ideas!
| import java.util.Map; | ||
| import java.util.Objects; | ||
|
|
||
| class EventEmitterWithData { |
There was a problem hiding this comment.
It would be good to have a unit test for this class either in this or next PRs.
…ys and added unit test
No description provided.