Skip to content

Commit

Permalink
[SDK] Allow metric instrument names to contain / characters (#2310)
Browse files Browse the repository at this point in the history
  • Loading branch information
marcalff authored Sep 14, 2023
1 parent b9776d6 commit 44096c8
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 5 deletions.
12 changes: 7 additions & 5 deletions sdk/src/metrics/instrument_metadata_validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ namespace sdk
{
namespace metrics
{
// instrument-name = ALPHA 0*254 ("_" / "." / "-" / ALPHA / DIGIT)
const std::string kInstrumentNamePattern = "[a-zA-Z][-_.a-zA-Z0-9]{0,254}";
// instrument-name = ALPHA 0*254 ("_" / "." / "-" / "/" / ALPHA / DIGIT)
const std::string kInstrumentNamePattern = "[a-zA-Z][-_./a-zA-Z0-9]{0,254}";
//
const std::string kInstrumentUnitPattern = "[\x01-\x7F]{0,63}";
// instrument-unit = It can have a maximum length of 63 ASCII chars
Expand Down Expand Up @@ -49,9 +49,11 @@ bool InstrumentMetaDataValidator::ValidateName(nostd::string_view name) const
{
return false;
}
// subsequent chars should be either of alphabets, digits, underscore, minus, dot
return !std::any_of(std::next(name.begin()), name.end(),
[](char c) { return !isalnum(c) && c != '-' && c != '_' && c != '.'; });
// subsequent chars should be either of alphabets, digits, underscore,
// minus, dot, slash
return !std::any_of(std::next(name.begin()), name.end(), [](char c) {
return !isalnum(c) && (c != '-') && (c != '_') && (c != '.') && (c != '/');
});
#endif
}

Expand Down
2 changes: 2 additions & 0 deletions sdk/test/metrics/instrument_metadata_validator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ TEST(InstrumentMetadataValidator, TestName)
"123€AAA€BBB", // unicode characters
"/\\sdsd", // string starting with special character
"***sSSs", // string starting with special character
"a\\broken\\path", // contains backward slash
CreateVeryLargeString(25) + "X", // total 256 characters
CreateVeryLargeString(26), // string much bigger than 255 characters
};
Expand All @@ -37,6 +38,7 @@ TEST(InstrumentMetadataValidator, TestName)
"s123", // starting with char, followed by numbers
"dsdsdsd_-.", // string , and valid nonalphanumeric
"d1234_-sDSDs.sdsd344", // combination of all valid characters
"a/path/to/some/metric", // contains forward slash
CreateVeryLargeString(5) + "ABCERTYG", // total 63 characters
CreateVeryLargeString(5) + "ABCERTYGJ", // total 64 characters
CreateVeryLargeString(24) + "ABCDEFGHI", // total 254 characters
Expand Down

2 comments on commit 44096c8

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 44096c8 Previous: b9776d6 Ratio
BM_LockFreeBuffer/4 8784730.434417725 ns/iter 4243996.018677755 ns/iter 2.07

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp api Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 44096c8 Previous: b9776d6 Ratio
BM_NaiveSpinLockThrashing/2/process_time/real_time 1.2082503392146184 ms/iter 0.2565367267601985 ms/iter 4.71

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.