fix(sdk-metrics): metric names should be case-insensitive #4059
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4059 +/- ##
==========================================
- Coverage 92.39% 91.89% -0.51%
==========================================
Files 321 320 -1
Lines 9282 9248 -34
Branches 1973 1961 -12
==========================================
- Hits 8576 8498 -78
- Misses 706 750 +44
|
| descriptor.unit === otherDescriptor.unit && | ||
| descriptor.type === otherDescriptor.type && | ||
| descriptor.valueType === otherDescriptor.valueType | ||
| ); | ||
| } | ||
|
|
||
| const NAME_REGEXP = /^[a-z][a-z0-9_.-]{0,62}$/i; |
There was a problem hiding this comment.
Is always good to include a comment with the description of the regex expression for easier reading
// ASCII string with a length no greater than 63 characters
There was a problem hiding this comment.
Names were increased to <=255 character length open-telemetry/opentelemetry-specification#3648
| const NAME_REGEXP = /^[a-z][a-z0-9_.-]{0,62}$/i; | |
| // ASCII string with a length no greater than 255 characters | |
| const NAME_REGEXP = /^[a-z][a-z0-9_.-]{0,255}$/i; |
packages/sdk-metrics/src/Meter.ts
Outdated
| @@ -50,6 +52,7 @@ export class Meter implements IMeter { | |||
| * Create a {@link Histogram} instrument. | |||
| */ | |||
| createHistogram(name: string, options?: MetricOptions): Histogram { | |||
| this._warnIfInvalidName(name); | |||
There was a problem hiding this comment.
Is it possible to add the warning during the call to createInstrumentDescriptor?
|
@legendecas will you have time to make the changes or would it be easier for me to take this over? |
# Conflicts: # CHANGELOG.md
|
@dyladan sorry for the delay. Updated! |
Which problem is this PR solving?
Fixes #4057
Short description of the changes
Type of change
How Has This Been Tested?
isDescriptorCompatibleWithChecklist: