beam-telemetry / telemetry

Dynamic dispatching library for metrics and instrumentations.
https://hexdocs.pm/telemetry
Apache License 2.0
872 stars 66 forks source link

span/3 StartMetadata and StopMetadata semantics #77

Closed davydog187 closed 3 years ago

davydog187 commented 3 years ago

I was working on adding some new telemetry events to Commanded to be used in the bridge library I'm developing for Otel. A point of confusion came for me when I needed to pass the metadata to :telemetry.span/3. In the PR, you'll see that I passed data metadata to the StartMetadata, but then passed an empty map for the StopMetadata.

After ruminating on this with @bryannaegele, it became obvious to me that I should be passing the same value to both start and stop. However, the documentation did not point me in this direction, and the API was surprising initially. I would have expected the StopMetadata to be merged into the StartMetadata.

I'm opening this issue to start a discussion on whether this needs a simple documentation update, or if the API needs to be reconsidered slightly.

josevalim commented 3 years ago

Definitely doc update because it is easier for you as the event emitter to add additional info rather than us always merging which is more expensive. :)