Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-valued metric tags not supported #3691

Closed
elramen opened this issue Jun 5, 2024 · 4 comments · Fixed by getsentry/develop#1307
Closed

Multi-valued metric tags not supported #3691

elramen opened this issue Jun 5, 2024 · 4 comments · Fixed by getsentry/develop#1307
Assignees

Comments

@elramen
Copy link

elramen commented Jun 5, 2024

The metrics developer docs say "metrics are tagged by key value pairs, where each key can have more than one value." Also the RFC says "More than one value per key is permitted if represented as a list or tuple." Multi-valued tags is also implemented in the python sdk (not sure about the other sdks).

However, it seems that relay doesn't allow multi-valued tags for single emissions, only the last value of the list/tuple sent is actually used. Have raised this issue offline but opening it here so it's not forgotten.

@jjbayer
Copy link
Member

jjbayer commented Jun 11, 2024

@elramen thanks, good catch! Do you have a specific use case for multi-value tags? Neither Relay nor storage support it so I will remove it from the Relay docs. As for the Python SDK docs, all the examples here show dictionaries so I think we're good?

jjbayer added a commit to getsentry/develop that referenced this issue Jun 11, 2024
Neither Relay nor storage support multi-value tags.

Fixes getsentry/relay#3691.
@elramen
Copy link
Author

elramen commented Jun 11, 2024

Hey @jjbayer, I don't have a specific use case for it, just opened this issue to make sure things are aligned.

I'll remove Tuple and List from the Python SDK's type hints for tag values.

@szokeasaurusrex
Copy link
Member

@elramen thanks, good catch! Do you have a specific use case for multi-value tags? Neither Relay nor storage support it so I will remove it from the Relay docs. As for the Python SDK docs, all the examples here show dictionaries so I think we're good?

@jjbayer To be clear, I believe that @elramen is referring to the tag values (i.e. the values of the tags dictionary), which in the Python docs examples are all strings. The question is referring to whether the tags could have multiple values, e.g. by setting a value in the dictionary to a list. The tags would still be defined in a dictionary

@Dav1dde
Copy link
Member

Dav1dde commented Jun 12, 2024

A single metric emission can only have one value per key and every key must be unique. Please check the docs PR if that new explanation does make sense to you.

elramen added a commit to getsentry/sentry-python that referenced this issue Jun 12, 2024
Remove Tuple and List from the MetricTagValue type as these are not supported and might confuse the user. See getsentry/relay#3691 for more information.
arjennienhuis pushed a commit to arjennienhuis/sentry-python that referenced this issue Sep 30, 2024
Remove Tuple and List from the MetricTagValue type as these are not supported and might confuse the user. See getsentry/relay#3691 for more information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants