Metric tag values not working as expected

[elramen]

How do you use Sentry?

Sentry Saas (sentry.io)

Version

2

Steps to Reproduce

1. Setup the SDK and type sentry_sdk.metrics.X where X is gauge, increment, set, or distribution. Hover X to see Optional[MetricTags] as type for tags. Go to the definition of MetricTags to see its definition: Mapping[str, MetricTagValue]. Go to the definition of MetricTagValue to see that it's defined as a Union of many types, including None, List, and Tuple.

2. Send a metric using a tag value of type None, List, or Tuple, e.g., sentry_sdk.metrics.gauge("test", 1, tags={"mytag": None}).

Expected Result

Metric shows up in sentry.io dashboard with tags attached as follows:

• None: If tag value is None, the tag is attached to the metric but has no tag value.
• List/Tuple: If tag value is a List or Tuple, the tag is attached to the metric and has all values specified in the List/Tuple.

Actual Result

None: tag isn't added.
List/Tuple: only the last value in the List/Tuple shows up as a value for the tag.

[elramen]

After briefly looking into this, I think the None case is as expected, i.e. tags aren't meant to have no value. If this is correct, this could be solved by removing None from the tag value type hint to not confuse the user.

Regarding the List and Tuple case, the behaviour seems to be a bug.

[elramen]

After further investigation:

Relay's implementation doesn't currently allow multi-valued tags for a single emission (i.e. List or Tuple as the tag value). However, this goes against what is written in the docs, the python implementation, and the metrics RFC. Therefore, it's probably a good idea to discuss which option we should go with to keep things consistent. @iambriccardo @jan-auer @mitsuhiko

[szokeasaurusrex]

Relay's implementation doesn't currently allow multi-valued tags for a single emission (i.e. List or Tuple as the tag value). However, this goes against what is written in the docs, the python implementation, and the metrics RFC. Therefore, it's probably a good idea to discuss which option we should go with to keep things consistent. @iambriccardo @jan-auer @mitsuhiko

@elramen, have you opened an issue to document this behavior in Relay's repository yet? If this is a Relay problem, the issue should live in that repo, not in the sentry-python repo

[elramen]

@szokeasaurusrex I haven't opened an issue in the relay repo. Not sure if it's a relay problem since it depends on what we want regarding this

[elramen]

Opened getsentry/relay#3691, closing this