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

Metric units should be treated as opaque strings #2793

Closed
evan-bradley opened this issue Jun 30, 2022 · 4 comments · Fixed by #2796
Closed

Metric units should be treated as opaque strings #2793

evan-bradley opened this issue Jun 30, 2022 · 4 comments · Fixed by #2796
Assignees
Labels
bug Something isn't working metrics

Comments

@evan-bradley
Copy link

Describe your environment
OpenTelemetry Python API and SDK versions 1.12.0rc1 running under Python 3.10.

Steps to reproduce
Create an instrument with a unit containing characters that are not alphanumeric.

from opentelemetry import metrics

meter = metrics.get_meter("server_meter")

meter.create_counter(
    name="request_count",
    description="Count the number of handled requests.",
    unit="{requests}",
)

What is the expected behavior?
The unit validation should only check that the unit has fewer than 63 ASCII characters.

What is the actual behavior?
Units are restricted to the following regex character set: [a-zA-Z0-9_].

An exception is thrown with the following message:

Expected ASCII string of maximum length 63 characters but got {requests}

Additional context
The unit validation regex being used: https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/metrics/_internal/instrument.py#L43

The API spec advises that units should be treated as opaque strings and only the length should be validated: https://opentelemetry.io/docs/reference/specification/metrics/api/#instrument-unit

Python defines \w as [a-zA-Z0-9_] when the ASCII flag is used: https://docs.python.org/3/library/re.html#regular-expression-syntax

@evan-bradley evan-bradley added the bug Something isn't working label Jun 30, 2022
@aabmass
Copy link
Member

aabmass commented Jun 30, 2022

@evan-bradley thanks for clear bug report. Any change you'd be willing to send a PR?

@ocelotl ocelotl added the metrics label Jul 1, 2022
@ocelotl ocelotl self-assigned this Jul 1, 2022
@ocelotl
Copy link
Contributor

ocelotl commented Jul 1, 2022

I'll work on this one 👍

ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jul 1, 2022
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this issue Jul 4, 2022
ocelotl added a commit that referenced this issue Jul 4, 2022
* Fix unit and name regexes

Fixes #2793

* Add changelog entry
@kalkibhagavanmeda
Copy link

Hi All
I have one question what need to do further if metric name is greater than 63 characters

@rohitkrishna-marneni
Copy link

@kalkibhagavanmeda Were you able to figure this out?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working metrics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants