develop icon indicating copy to clipboard operation
develop copied to clipboard

Document Span Descriptions

Open philipphofmann opened this issue 4 years ago • 5 comments

With https://github.com/getsentry/develop/pull/452 we specify the operations for spans. To align the descriptions for the spans across SDKs we should also document them.

For file spans, we already discussed that filename + bytes count file.txt (203 KB) should go in the description and the full file path should go into data of the span as data["filepath"] = "/absolute/example/file.txt". Should the bytes count be KB, MB, GB, ... ?

philipphofmann avatar Nov 17 '21 13:11 philipphofmann

Leaving the general discussion of descriptions aside....

Should the bytes count be KB, MB, GB, ... ?

Possible paths I see:

  1. Leave this up to the implementation. In some languages/platforms it is easy and convenient to pretty-print units, in others it would require extra work that may not justify itself.
    • Pros: less code in SDKs
    • Cons: possibly inconsistent user experience.
  2. Always report bytes. Leave it up to the UI to parse and pretty-print for all platforms equally.
    • Pros: uniform user experience.
    • Cons: the UI shouldn't need to parse an arbitrary span.description string to format file sizes. If we cared to communicate data at this level, we should probably do it with specific metadata fields that are simpler to read from and then possibly augment the description / span widget.
  3. Report file size elsewhere, e.g. span.data.file.size -- new set of conventions for metadata.
    • Pros: uniform user experience, easier to transport data from SDKs to Sentry UI, only one place needs to pretty-print units.
    • Cons: more specs (?)

If we care enough to report the size and want to make it a convention, I'm tempted to favor path (3) above. Reporting metadata in explicit fields would also allow to future extensibility, e.g. reporting file permissions, modification date, etc, as well as the same mechanism can be used for other things that are not files.

Trying to stuff information in the description seems like a bad pattern (harder to parse, validate, store, query, expand and evolve).

rhcarvalho avatar Nov 18 '21 10:11 rhcarvalho

Thanks for the input, @rhcarvalho. I am also in favor of (3), but I don't fully understand why the con is a con?

philipphofmann avatar Nov 18 '21 11:11 philipphofmann

but I don't fully understand why the con is a con?

😄 because I make mistakes 😓

Edited.

rhcarvalho avatar Nov 18 '21 14:11 rhcarvalho

JFYI in https://github.com/getsentry/sentry-java/pull/1826 we went for:

  • Filename + pretty-printed byte count as description (e.g. file.txt (12 kB))
  • Absolute path as data.file.path (e.g. /Users/{user}/workspace/file.txt). Note that we only send absolute path in case sendDefaultPii is enabled OR if the SDK is running on a mobile device (there the absolute path does not contain any PII)
  • Raw byte count as data.file.size (e.g. 1235678)

cc @brustolin

romtsn avatar Dec 09 '21 17:12 romtsn

Also while typing, I realized that file.size is probably not ideal, because if we mean the byte count read/written, it's not guaranteed to be the file size

romtsn avatar Dec 09 '21 17:12 romtsn