fix(metrics): accurate error messages for instrument name and unit validation by ersalil · Pull Request #5283 · open-telemetry/opentelemetry-python

ersalil · 2026-06-09T21:35:33Z

Summary

Use two distinct validation error messages for metrics instrument creation so name and unit failures reflect their different spec constraints.

Fix #5284

Changes

Added _NAME_ERROR_MESSAGE and _UNIT_ERROR_MESSAGE in opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py
Updated instrument validation raises to use the correct message for:
- invalid instrument names: ASCII name, starts with a letter, allowed chars, max 255 characters
- invalid units: ASCII-only string, max 63 characters
Added regression tests in opentelemetry-sdk/tests/metrics/test_instrument_error_message.py

Tests

test_invalid_name_error_message verifies invalid names mention “start with a letter” and max length 255
test_invalid_unit_error_message verifies invalid units mention ASCII and max length 63

Notes

Message-only fix; validation logic and exception type remain unchanged
Kept exception type as Exception to avoid changing behavior in this small fix; happy to change to ValueError if maintainers prefer

…t name and unit validation; add tests

linux-foundation-easycla · 2026-06-09T21:35:42Z

✅ login: Copilot / name: Copilot Autofix powered by AI (f3b3f07)
✅ login: ersalil / name: Salil Agrawal (f3b3f07)
✅ login: ersalil / name: ersalil (ad23ad0, e62c5ba, ff034b2)
✅ login: lzchen / name: Leighton Chen (e332d9f)
❌ The email address for the commit (e62c5ba) is not linked to the GitHub account, preventing the EasyCLA check. Consult this Help Article and GitHub Help to resolve. (To view the commit's email address, add .patch at the end of this PR page's URL.) For further assistance with EasyCLA, please visit our EasyCLA portal and chat with our support bot.

One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via:

Co-authored-by: name <email>

Supported Co-authored-by: formats include:

Anything <id+login@users.noreply.github.com> - it will locate your GitHub user by id part.
Anything <login@users.noreply.github.com> - it will locate your GitHub user by login part.
Anything <public-email> - it will locate your GitHub user by public-email part. Note that this email must be made public on Github.
Anything <other-email> - it will locate your GitHub user by other-email part but only if that email was used before for any other CLA as a main commit author.
login <any-valid-email> - it will locate your GitHub user by login part, note that login part must be at least 3 characters long.

Alternatively, if the co-author should not be included, remove the Co-authored-by: line from the commit message.

Please update your commit message(s) by doing git commit --amend and then git push [--force] and then request re-running CLA check via commenting on this pull request:

/easycla

aabmass · 2026-06-11T03:57:21Z

@ersalil can you please confirm the actual code is doing the validation you expect, and if you're able to link to the spec or spec matrix for this, it would be helpful.

Copilot

Pull request overview

This PR updates the SDK metrics instrument construction path to emit distinct, spec-accurate validation error messages for invalid instrument names vs invalid units, and adds regression coverage to prevent message mix-ups in the future.

Changes:

Introduced dedicated constants for name vs unit validation error messages and used them in synchronous/asynchronous instrument initialization.
Added regression tests asserting key spec constraints are reflected in the raised exception messages.
Added a changelog fragment documenting the name error message clarification.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
`opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py`	Split validation error messaging so name/unit failures raise different messages.
`opentelemetry-sdk/tests/metrics/test_instrument_error_message.py`	New regression tests to assert name/unit validation errors contain correct spec wording.
`.changelog/5283.changed`	Changelog entry describing the updated name validation error message.

Comments suppressed due to low confidence (1)

.changelog/5283.changed:2

This changelog fragment has an extra blank line; other fragments in .changelog/*.changed are single-line entries. Please remove the trailing empty line so the fragment is exactly one line.

metrics: clarify instrument name validation error message to reflect spec constraints (start with a letter, allowed chars, max length 255)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ersalil · 2026-06-11T04:40:53Z

@aabmass Thanks for the review! Here's comprehensive validation:

Validation Code

The validation happens in the API layer:
https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/metrics/_internal/instrument.py#L27-L28

_name_regex = re_compile(r"[a-zA-Z][-_./a-zA-Z0-9]{0,254}")
_unit_regex = re_compile(r"[\x00-\x7F]{0,63}")

Spec Reference

OpenTelemetry Metrics API Specification confirms:

Instrument Name: Must start with letter, contain ASCII alphanumerics + _, ., -, /, max 255 chars
Unit: ASCII-only, opaque string, max 63 chars

Test Results

Unit Tests

test_invalid_name_error_message PASSED [ 50%]
test_invalid_unit_error_message PASSED [100%]
============================== 2 passed in 0.08s ===============================

Local verification, name vs unit error-message outputs

Test 1: Invalid name (starts with number)

Error: Instrument name must be an ASCII string, start with a letter, 
contain only letters, digits, '_', '.', '-', '/' and be at most 255 characters; got 1invalid

Test 2: Invalid unit (non-ASCII)

Error: Expected ASCII string of maximum length 63 characters but got Ñ

Test 3: Unit exceeds 63 chars

Error: Expected ASCII string of maximum length 63 characters but got aaa...

All tests passed.

Distinguishes name vs unit validation failures
Reflects actual spec constraints in error messages

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

+if __name__ == "__main__":
+    unittest.main()


+            _Synchronous("1invalid", scope, object())
+        msg = str(cm.exception)
+        self.assertIn("start with a letter", msg)
+        self.assertIn("255", msg)


+_NAME_ERROR_MESSAGE = (
+    "Instrument name must be an ASCII string, start with a letter, "
+    "contain only letters, digits, '_', '.', '-', '/' and be at most 255 characters; got {}"
+)
+
+_UNIT_ERROR_MESSAGE = (
    "Expected ASCII string of maximum length 63 characters but got {}"
 )


xrmx · 2026-06-17T14:43:38Z

@ersalil the copilot review seems sensible, please take a look at the review comments

herin049 · 2026-06-18T01:59:49Z

@@ -0,0 +1 @@
+metrics: clarify instrument name validation error message to reflect spec constraints (start with a letter, allowed chars, max length 255)


Please include the full package name here.

herin049 · 2026-06-18T01:59:59Z

+        self.assertIn("ASCII", msg)
+
+
+if __name__ == "__main__":


Please remove this.

- changelog: use full package name and cover both name and unit validation - instrument.py: use {!r} when formatting offending value; make unit message reference the unit field for parity with the name message - tests: assert precise "at most 255 characters" substring and drop the unnecessary unittest.main() entrypoint Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

ersalil · 2026-06-30T15:11:41Z

Thanks for the reviews @xrmx @herin049 and the Copilot bot! Addressed all the feedback in e62c5ba:

Changelog (@herin049): now uses the full package name and follows the `opentelemetry-sdk`: convention; reworded to cover both name and unit validation. The trailing blank line is also gone (single-line fragment).
unittest.main() entrypoint (@herin049 / Copilot): removed.
assertIn("255") (Copilot): tightened to assertIn("at most 255 characters", ...) so it can't false-positive on unrelated numbers.
Value formatting (Copilot): both _NAME_ERROR_MESSAGE and _UNIT_ERROR_MESSAGE now format the offending value with {!r}, which disambiguates whitespace/empty/non-string inputs. The unit message also now reads Instrument unit must be an ASCII string of maximum length 63 characters; got {!r} for parity with the name message.
Import grouping (Copilot): the opentelemetry.* imports are sorted into a single block; ruff check passes.

Tests pass locally (test_invalid_name_error_message, test_invalid_unit_error_message).

fix(metrics): provide distinct, accurate error messages for instrumen…

ff034b2

…t name and unit validation; add tests

ersalil requested a review from a team as a code owner June 9, 2026 21:35

otelbot-python Bot added this to Python PR digest Jun 9, 2026

chore(changelog): add fragment for PR open-telemetry#5283

ad23ad0

aabmass requested a review from Copilot June 11, 2026 03:57

Copilot started reviewing on behalf of aabmass June 11, 2026 03:57 View session

aabmass moved this to Ready for review in Python PR digest Jun 11, 2026

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Comment thread opentelemetry-sdk/tests/metrics/test_instrument_error_message.py Outdated

Potential fix for pull request finding

f3b3f07

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

ersalil requested a review from Copilot June 11, 2026 07:23

Copilot AI reviewed Jun 11, 2026

View reviewed changes

herin049 reviewed Jun 18, 2026

View reviewed changes

xrmx moved this from Ready for review to Reviewed PRs that need fixes in Python PR digest Jun 18, 2026

lzchen and others added 2 commits June 18, 2026 10:43

Merge branch 'main' into fix/metrics-instrument-error-message

e332d9f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(metrics): accurate error messages for instrument name and unit validation#5283

fix(metrics): accurate error messages for instrument name and unit validation#5283
ersalil wants to merge 5 commits into
open-telemetry:mainfrom
ersalil:fix/metrics-instrument-error-message

ersalil commented Jun 9, 2026 •

edited by xrmx

Loading

Uh oh!

linux-foundation-easycla Bot commented Jun 9, 2026 •

edited

Loading

Uh oh!

aabmass commented Jun 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

ersalil commented Jun 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

xrmx commented Jun 17, 2026

Uh oh!

herin049 Jun 18, 2026

Uh oh!

herin049 Jun 18, 2026

Uh oh!

ersalil commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		@@ -0,0 +1 @@
		metrics: clarify instrument name validation error message to reflect spec constraints (start with a letter, allowed chars, max length 255)

Uh oh!

Conversation

ersalil commented Jun 9, 2026 • edited by xrmx Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Tests

Notes

Uh oh!

linux-foundation-easycla Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aabmass commented Jun 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

ersalil commented Jun 11, 2026

Validation Code

Spec Reference

Test Results

Unit Tests

Local verification, name vs unit error-message outputs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

xrmx commented Jun 17, 2026

Uh oh!

herin049 Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

herin049 Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

ersalil commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ersalil commented Jun 9, 2026 •

edited by xrmx

Loading

linux-foundation-easycla Bot commented Jun 9, 2026 •

edited

Loading