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

dog_stats_api doesn't encode pipe char #19

Closed
yannmh opened this issue Apr 3, 2015 · 7 comments
Closed

dog_stats_api doesn't encode pipe char #19

yannmh opened this issue Apr 3, 2015 · 7 comments
Assignees
Labels
kind/bug Bug related issue resource/dogstatsd severity/minor Minor severity issue

Comments

@yannmh
Copy link
Member

yannmh commented Apr 3, 2015

Issue by clofresh
Friday Feb 21, 2014 at 23:23 GMT
Originally opened as https://github.com/DataDog/dogapi/issues/81


| is used by the statsd protocol to delimit the fields, so if you use it in a tag, you'll lose whatever's after the pipe

@yannmh yannmh added the bug label Apr 3, 2015
@github-actions github-actions bot added the stale Stale - Bot reminder label Jan 8, 2020
@gzussa gzussa removed the bug label Jan 17, 2020
@jirikuncar jirikuncar added kind/bug Bug related issue resource/dogstatsd severity/minor Minor severity issue and removed stale Stale - Bot reminder labels Jan 21, 2020
@jirikuncar
Copy link
Contributor

jirikuncar commented Jan 21, 2020

@DataDog/agent-core should we escape or remove | in tag?

Is there any documentation which specifies valid tags? I have only found https://docs.datadoghq.com/developers/metrics/dogstatsd_metrics_submission/?tab=python#metric-tagging.

see also #517

@DataDog DataDog deleted a comment from github-actions bot Jan 21, 2020
@arbll
Copy link
Member

arbll commented Jan 21, 2020

@jirikuncar https://docs.datadoghq.com/tagging/#defining-tags defines valid characters for tags. AFAIK statsd/dogstatsd do not specify how to escape characters that conflict with the protocol or are invalid. It probably make sense to either strip them or replace them by a valid character like _.

In any case let's make sure we align with other clients if they have logic to escape tags / metric names.

@jirikuncar
Copy link
Contributor

@arbll thank you for the link. It doesn't say how the tags should be encoded in case of dogstatd. We can add a validation, but | seems like a valid character.

@arbll
Copy link
Member

arbll commented Jan 22, 2020

@jirikuncar | is invalid for all datadog tags, so it's also invalid for dogstatsd.
Again, I don't think dogstatsd/statsd escaping is defined anywhere. See statsd/statsd#585 for example

@jirikuncar
Copy link
Contributor

jirikuncar commented Jan 22, 2020

@arbll so basically a tag has to match ^[A-Za-z][A-Za-z0-9_\-:,/]{0,199}$ after re.sub('[^A-Za-z0-9_\-:,/]', '_', tag), right?


Playground: https://regex101.com/r/yJ8qva/1/tests

@github-actions
Copy link

Thanks for your contribution!

This issue has been automatically marked as stale because it has not had activity in the last 30 days. Note that the issue will not be automatically closed, but this notification will remind us to investigate why there's been inactivity. Thank you for participating in the Datadog open source community.

If you would like this issue to remain open:

  1. Verify that you can still reproduce the issue in the latest version of this project.

  2. Comment that the issue is still reproducible and include updated details requested in the issue template.

@github-actions github-actions bot added the stale Stale - Bot reminder label Feb 23, 2020
jirikuncar added a commit to jirikuncar/datadogpy that referenced this issue Feb 28, 2020
@zippolyte
Copy link
Contributor

zippolyte commented Mar 3, 2020

closed in #517

The library will now sanitize tags before sending them

@zippolyte zippolyte removed the stale Stale - Bot reminder label Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Bug related issue resource/dogstatsd severity/minor Minor severity issue
Projects
None yet
Development

No branches or pull requests

7 participants