Add Event and TimedEvent

[danielkhan]

This adds a type for Event and TimedEvent needed for SpanData.

[draffensperger]

What's the motivation for making Event and TimedEvent distinct? Is that specified in the spec?

[danielkhan]

Not explicitly. Here's the spec that mentions them and here is the Java implementation.

A timed event has a timestamp attached to it, so to me, it makes sense to create a type for it and I think that was also the reason why Java does it like that.

[rochdev]

I don't think there needs to be a distinction. An event always has a time. This is a fact not only in software but also in the physical world.

[danielkhan]

Event does not have a time and there is a TimedEvent in the spec. I think that we should stick to the spec and discuss and deviation from it with the spec team.

[rochdev]

There are so many things wrong with the spec that I don't think this is the best approach.

[bg451]

Yeah I am confused why there would be an event with no time, seems like a leftover from opencensus's implementation that ended up in the API.

[mayurkale22]

@danielkhan The Event interface has already been addressed in PR #63. Could you please review ?

[danielkhan]

@danielkhan The Event interface has already been addressed in PR #63. Could you please review ?

I missed that one. Done. I'll wait for this to be merged and modify my PR then.

[draffensperger]

Could we do something like this:

interface Event {
  name: string;
  attributes: Attributes;
}
interface TimeEvent extends Event {
  time: number;
}

?

That would allows the events to be simple structures and the interfaces could show a relationship between the two.

Could we always use TimeEvent in the actual implementation, in which case the Event could just exist as a nod to the spec? Or are there cases where we would actually want an Event without a time?

[danielkhan]

Changed the types to extend Event to create a TimedEvent

[rochdev]

Actually I think I understand the rationale. If I'm not mistaken, Event would allow you to create the TimedEvent (which is the real event). Basically you pass an Event to some kind of event builder that will create the actual TimedEvent for you with the proper timestamp.

This would not be an issue if the API was designed differently, but unfortunately that is not the case right now.

[rochdev]

File naming is inconsistent with the rest of the project.

[danielkhan]

File naming is inconsistent with the rest of the project.

Please suggest because as far as I can see, there are still discussions and the project is inconsistent. Can we merge and sort that out with #38?

[codecov-io]

Codecov Report

Merging #73 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master    #73   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          10     10           
  Lines         152    198   +46     
  Branches        8      8           
=====================================
+ Hits          152    198   +46

Impacted Files	Coverage Δ
test/trace/tracestate.test.ts	100% <0%> (ø)	⬆️
test/trace/ProbabilitySampler.test.ts	100% <0%> (ø)	⬆️
test/resources/resource.test.ts	100% <0%> (ø)	⬆️
src/common/util/id.ts	100% <0%> (ø)	⬆️
test/common/util.id.test.ts	100% <0%> (ø)	⬆️
test/trace/NoopSpan.test.ts	100% <0%> (ø)	⬆️