Span Event should have a timestamp

[hekike]

Events should have a timestamp:
https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-types/src/trace/Event.ts

and span.addEvent(..) should set it:
https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-basic-tracer/src/Span.ts#L80

Related spec:
https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#add-events

[mayurkale22]

We already have an interface called TimedEvent, which represents a timed event. https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-types/src/trace/TimedEvent.ts#L23

[rochdev]

I think the idea is to merge both. The spec doesn't mention having 2 classes, and returning an Event from addEvent with a timestamp on it makes a lot more sense since it cannot happen that an event didn't occur at a specific point in time.

[danielkhan]

@rochdev timedEvent comes from here: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#gettimedevents

But looking at this https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#add-events it looks as if the time is also specified on a regular event.

Looking at this, I find the sentence

Return the collection of Events with the timestamps associated with this SpanData

rather confusing.

[rochdev]

If addEvents adds an Event with a timestamp, and getTimedEvents returns these Event types, then it matches exactly the specification.