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

metrics: Define name syntax #3643

Closed
wants to merge 2 commits into from

Conversation

pellared
Copy link
Member

@pellared pellared commented Aug 7, 2023

Fixes #3579

Changes

The existing meter name validation is "underpowered" compared to instrument name validation.

I propose to align and use the same syntax and behavior for both names.

Why

I am not sure what are the exact reasons that are behind the existing instrument name syntax, but I guess the same reasons could be applies to meter name syntax. I think it would make the specification more consistent.

@pellared pellared changed the title Define name syntax [metrics] Define name syntax Aug 7, 2023
@pellared pellared changed the title [metrics] Define name syntax metrics: Define name syntax Aug 7, 2023
@pellared pellared marked this pull request as ready for review August 7, 2023 11:55
@pellared pellared requested review from a team August 7, 2023 11:55
@pellared
Copy link
Member Author

pellared commented Aug 7, 2023

CC @open-telemetry/go-maintainers

```

* They are not null or empty strings.
* They are case-insensitive, ASCII strings.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will introduce a similar issue to this. I would recommend the meter name syntax not be case-insensitive.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand correctly that in order to address this comment I would need to introduce a dedicated section for meter name instead of unifying the name syntax (as we should not make the existing instrument name case-sensitive)?

@pellared
Copy link
Member Author

pellared commented Aug 8, 2023

Closing per #3579 (comment). @jack-berg thanks for your comment 👍

@pellared pellared closed this Aug 8, 2023
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 this pull request may close these issues.

Meter name validation
3 participants