- 
                Notifications
    
You must be signed in to change notification settings  - Fork 501
 
Add bundle version of utf8_range to validate attributes #3512
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
base: main
Are you sure you want to change the base?
Conversation
          ✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.
  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a bundled UTF-8 validation library (utf8_range) and integrates it into attribute validation throughout the SDK, ensuring that span, resource, and instrumentation-scope attributes containing invalid UTF-8 are filtered out with warnings.
- Add 
attribute_validitymodule wrapping the newutf8_rangelibrary - Integrate attribute validation/filtering into 
Span,Resource, andInstrumentationScope - Add build rules for 
utf8_rangeand update CMake/Bazel files accordingly 
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description | 
|---|---|
| sdk/src/common/internal/utf8_range/uft8_range.cc | Added bundled UTF-8 validation implementation | 
| sdk/src/common/internal/utf8_range/utf8_range.h | Declared public API for UTF-8 validation (IsValid, ValidPrefix) | 
| sdk/src/common/attribute_validity.cc | Wrapped validation logic around SDK attribute values | 
| sdk/src/common/BUILD | Added utf8_range and attribute_validity libraries | 
| sdk/src/common/CMakeLists.txt | Updated to include new source in common build | 
| sdk/src/trace/span.cc | Applied attribute validation in Span::SetAttribute and AddLink | 
| sdk/src/resource/resource.cc | Applied attribute validation in Resource constructor | 
| sdk/include/opentelemetry/sdk/instrumentationscope/instrumentation_scope.h | Applied attribute validation in instrumentation scope factories and setters | 
| sdk/test/instrumentationscope/CMakeLists.txt | Updated linkage to include opentelemetry_common | 
| sdk/test/instrumentationscope/BUILD | Added dependency on attribute_validity for tests | 
Comments suppressed due to low confidence (2)
sdk/src/common/internal/utf8_range/uft8_range.cc:1
- The source file is named 
uft8_range.cc, which looks like a typo. Renaming it toutf8_range.ccwould align with the header (utf8_range.h) and improve consistency. 
// Copyright 2023 Google LLC
sdk/src/common/BUILD:6
- The new 
utf8_rangelibrary adds core validation logic but there are no accompanying unit tests verifying valid and invalid UTF-8 sequences. Consider adding tests undersdk/test/common/to cover edge cases. 
cc_library(
          Codecov Report❌ Patch coverage is  Additional details and impacted files@@            Coverage Diff             @@
##             main    #3512      +/-   ##
==========================================
- Coverage   90.00%   89.01%   -0.98%     
==========================================
  Files         225      228       +3     
  Lines        7105     7323     +218     
==========================================
+ Hits         6394     6518     +124     
- Misses        711      805      +94     
 🚀 New features to boost your workflow:
  | 
    
71e6ce2    to
    f02ac94      
    Compare
  
    f02ac94    to
    9fe92a8      
    Compare
  
    69c0259    to
    d0b4bfe      
    Compare
  
    f9e1da8    to
    c804ec7      
    Compare
  
    Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.30.9 to 4.31.0. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@16140ae...4e94bd1) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.31.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 5.0.0 to 6.0.0. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@634f93c...018cc2c) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: 6.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- prevent superfluous copy in construction of OwnedAttributeValue with std::string or span - combined identical lambdas of both lamdas - use const-ref - removed unneeded (stateless) members
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.31.0 to 4.31.2. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@4e94bd1...0499de3) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.31.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
c804ec7    to
    08e80f3      
    Compare
  
    …string_in_setters # Conflicts: # CHANGELOG.md
Fixes #3491
Changes
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes