beam-telemetry/telemetry

`:telemetry.span/3` documentation seems inconsistent

LostKobrakai opened this issue · 4 comments

When providing StartMetadata and StopMetadata, these values will be sent independently to start and stop events. […] In general, StopMetadata should only provide values that are additive to StartMetadata so that handlers, such as those used for metrics, can rely entirely on the stop event.

It's not really clear from this if StartMetadata values needs to be supplied as StopMetadata as well if they should be available for both events. The code seems to suggest they should, the docs seem to suggest to not do that.

We don't perform any merging, so indeed the docs are inaccurate. PRs welcome!

Hello! I'm still a tad confused about this, even after the linked PR got merged.

In the quoted documentation paragraph, the docs mention that a reporter should be able to rely entirely on the stop event. However, this doesn't seem possible if StartMetadata isn't merged (automatically) into StopMetadata, because there is possibly a large amount of metadata that is unavailable to the stop event by default, and must be manually merged into the stop metadata when a span wrapper is implemented.

This seems to me that it makes it very easy to implement telemetry in a way which makes reporters very arduous to write (with need for long-running state to track and match span-related events via their references). I'm unsure if this is the intent, or if the documentation is incorrect, or if I'm just misunderstanding things entirely.

I'm not sure why that isn't the behavior.

However, this doesn't seem possible if StartMetadata isn't merged (automatically) into StopMetadata,

we should document that it is extremely Recommended for callers to return the stop metadata as a superset of the start metadata.