Skip to content

Commit 4ba4995

Browse files
owentowent
committed
Implement non-utf8 string to bytes in OTLP exporters.
https://github.com/open-telemetry/opentelemetry-specification/blame/v1.55.0/specification/common/attribute-type-mapping.md
1 parent 7e181b6 commit 4ba4995

5 files changed

Lines changed: 187 additions & 32 deletions

File tree

‎cmake/protobuf.cmake‎

Lines changed: 68 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,37 @@
11
# Copyright The OpenTelemetry Authors
22
# SPDX-License-Identifier: Apache-2.0
33

4-
5-
# Import Protobuf targets (protobuf::libprotobuf and protobuf::protoc) and set PROTOBUF_PROTOC_EXECUTABLE.
6-
# 1. If gRPC was fetched from github then use the Protobuf submodule built with gRPC
7-
# 2. Find an installed Protobuf package
8-
# 3. Use FetchContent to fetch and build Protobuf from GitHub
9-
10-
if(DEFINED gRPC_PROVIDER AND NOT gRPC_PROVIDER STREQUAL "find_package" AND TARGET libprotobuf)
4+
# Import Protobuf targets (protobuf::libprotobuf and protobuf::protoc) and set
5+
# PROTOBUF_PROTOC_EXECUTABLE. 1. If gRPC was fetched from github then use the
6+
# Protobuf submodule built with gRPC 2. Find an installed Protobuf package 3.
7+
# Use FetchContent to fetch and build Protobuf from GitHub
8+
9+
if(DEFINED gRPC_PROVIDER
10+
AND NOT gRPC_PROVIDER STREQUAL "find_package"
11+
AND TARGET libprotobuf)
1112
# gRPC was fetched and built Protobuf as a submodule
1213

13-
set(_Protobuf_VERSION_REGEX "\"cpp\"[ \t]*:[ \t]*\"([0-9]+\\.[0-9]+(\\.[0-9]+)?)\"")
14-
set(_Protobuf_VERSION_FILE "${grpc_SOURCE_DIR}/third_party/protobuf/version.json")
14+
set(_Protobuf_VERSION_REGEX
15+
"\"cpp\"[ \t]*:[ \t]*\"([0-9]+\\.[0-9]+(\\.[0-9]+)?)\"")
16+
set(_Protobuf_VERSION_FILE
17+
"${grpc_SOURCE_DIR}/third_party/protobuf/version.json")
1518

1619
file(READ "${_Protobuf_VERSION_FILE}" _Protobuf_VERSION_FILE_CONTENTS)
1720
if(_Protobuf_VERSION_FILE_CONTENTS MATCHES ${_Protobuf_VERSION_REGEX})
1821
set(Protobuf_VERSION "${CMAKE_MATCH_1}")
1922
else()
20-
message(WARNING "Failed to parse Protobuf version from ${_Protobuf_VERSION_FILE} using regex ${_Protobuf_VERSION_REGEX}")
23+
message(
24+
WARNING
25+
"Failed to parse Protobuf version from ${_Protobuf_VERSION_FILE} using regex ${_Protobuf_VERSION_REGEX}"
26+
)
2127
endif()
2228
set(Protobuf_PROVIDER "grpc_submodule")
2329
else()
2430

25-
# Search for an installed Protobuf package explicitly using the CONFIG search mode first followed by the MODULE search mode.
26-
# Protobuf versions < 3.22.0 may be found using the module mode and some protobuf apt packages do not support the CONFIG search.
31+
# Search for an installed Protobuf package explicitly using the CONFIG search
32+
# mode first followed by the MODULE search mode. Protobuf versions < 3.22.0
33+
# may be found using the module mode and some protobuf apt packages do not
34+
# support the CONFIG search.
2735

2836
find_package(Protobuf CONFIG QUIET)
2937
set(Protobuf_PROVIDER "find_package")
@@ -36,45 +44,82 @@ else()
3644
FetchContent_Declare(
3745
"protobuf"
3846
GIT_REPOSITORY "https://github.com/protocolbuffers/protobuf.git"
39-
GIT_TAG "${protobuf_GIT_TAG}"
40-
)
41-
42-
set(protobuf_INSTALL ${OPENTELEMETRY_INSTALL} CACHE BOOL "" FORCE)
43-
set(protobuf_BUILD_TESTS OFF CACHE BOOL "" FORCE)
44-
set(protobuf_BUILD_EXAMPLES OFF CACHE BOOL "" FORCE)
47+
GIT_TAG "${protobuf_GIT_TAG}")
48+
49+
set(protobuf_INSTALL
50+
${OPENTELEMETRY_INSTALL}
51+
CACHE BOOL "" FORCE)
52+
set(protobuf_BUILD_TESTS
53+
OFF
54+
CACHE BOOL "" FORCE)
55+
set(protobuf_BUILD_EXAMPLES
56+
OFF
57+
CACHE BOOL "" FORCE)
4558

4659
FetchContent_MakeAvailable(protobuf)
4760

4861
set(Protobuf_PROVIDER "fetch_repository")
4962

5063
# Set the Protobuf_VERSION variable from the git tag.
51-
string(REGEX REPLACE "^v([0-9]+\\.[0-9]+\\.[0-9]+)$" "\\1" Protobuf_VERSION "${protobuf_GIT_TAG}")
64+
string(REGEX REPLACE "^v([0-9]+\\.[0-9]+\\.[0-9]+)$" "\\1" Protobuf_VERSION
65+
"${protobuf_GIT_TAG}")
5266

5367
if(TARGET libprotobuf)
54-
set_target_properties(libprotobuf PROPERTIES POSITION_INDEPENDENT_CODE ON CXX_CLANG_TIDY "" CXX_INCLUDE_WHAT_YOU_USE "")
68+
set_target_properties(
69+
libprotobuf
70+
PROPERTIES POSITION_INDEPENDENT_CODE ON
71+
CXX_CLANG_TIDY ""
72+
CXX_INCLUDE_WHAT_YOU_USE "")
5573
endif()
5674

5775
endif()
5876
endif()
5977

6078
if(NOT TARGET protobuf::libprotobuf)
61-
message(FATAL_ERROR "A required protobuf target (protobuf::libprotobuf) was not imported")
79+
message(
80+
FATAL_ERROR
81+
"A required protobuf target (protobuf::libprotobuf) was not imported")
6282
endif()
6383

6484
if(PROTOBUF_PROTOC_EXECUTABLE AND NOT Protobuf_PROTOC_EXECUTABLE)
65-
message(WARNING "Use of PROTOBUF_PROTOC_EXECUTABLE is deprecated. Please use Protobuf_PROTOC_EXECUTABLE instead.")
85+
message(
86+
WARNING
87+
"Use of PROTOBUF_PROTOC_EXECUTABLE is deprecated. Please use Protobuf_PROTOC_EXECUTABLE instead."
88+
)
6689
set(Protobuf_PROTOC_EXECUTABLE "${PROTOBUF_PROTOC_EXECUTABLE}")
6790
endif()
6891

6992
if(CMAKE_CROSSCOMPILING)
7093
find_program(Protobuf_PROTOC_EXECUTABLE protoc)
7194
else()
7295
if(NOT TARGET protobuf::protoc)
73-
message(FATAL_ERROR "A required protobuf target (protobuf::protoc) was not imported")
96+
message(
97+
FATAL_ERROR
98+
"A required protobuf target (protobuf::protoc) was not imported")
7499
endif()
75100
set(Protobuf_PROTOC_EXECUTABLE "$<TARGET_FILE:protobuf::protoc>")
76101
endif()
77102

78103
set(PROTOBUF_PROTOC_EXECUTABLE "${Protobuf_PROTOC_EXECUTABLE}")
79104

80105
message(STATUS "PROTOBUF_PROTOC_EXECUTABLE=${PROTOBUF_PROTOC_EXECUTABLE}")
106+
107+
# When protobuf is fetched or built as a gRPC submodule utf8_range is compiled
108+
# but may need the alias target created
109+
if(TARGET utf8_validity AND NOT TARGET utf8_range::utf8_validity)
110+
add_library(utf8_range::utf8_validity ALIAS utf8_validity)
111+
endif()
112+
113+
# For the find_package(Protobuf) path, utf8_range can also be found
114+
if(NOT TARGET utf8_range::utf8_validity)
115+
find_package(utf8_range CONFIG QUIET)
116+
endif()
117+
118+
# The utf8_range target should now be available for use in otel-cpp
119+
if(NOT TARGET utf8_range::utf8_validity)
120+
message(
121+
WARNING
122+
"Target (utf8_range::utf8_validity) was not found. "
123+
"We will not validate UTF-8 strings in the OTLP exporter."
124+
"Which may lead to record drops when sending non UTF-8 data as string.")
125+
endif()

‎exporters/otlp/BUILD‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,19 @@ load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark")
88

99
package(default_visibility = ["//visibility:public"])
1010

11+
# utf8_range from protobuf source tree for UTF-8 validation in OTLP
12+
# value conversion. This mirrors the CMake opentelemetry_otlp_utf8_validity
13+
# target.
14+
cc_library(
15+
name = "otlp_utf8_validity",
16+
defines = ["ENABLE_OTLP_UTF8_VALIDITY"],
17+
tags = ["otlp"],
18+
deps = [
19+
"@com_google_protobuf//third_party/utf8_range",
20+
"@com_google_protobuf//third_party/utf8_range:utf8_validity",
21+
],
22+
)
23+
1124
cc_library(
1225
name = "otlp_recordable",
1326
srcs = [
@@ -32,6 +45,7 @@ cc_library(
3245
strip_include_prefix = "include",
3346
tags = ["otlp"],
3447
deps = [
48+
":otlp_utf8_validity",
3549
"//sdk/src/logs",
3650
"//sdk/src/resource",
3751
"//sdk/src/trace",

‎exporters/otlp/CMakeLists.txt‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,16 @@ target_include_directories(
3333

3434
set(OPENTELEMETRY_OTLP_TARGETS opentelemetry_otlp_recordable)
3535

36-
target_link_libraries(opentelemetry_otlp_recordable PUBLIC opentelemetry_logs)
37-
target_link_libraries(opentelemetry_otlp_recordable
38-
PUBLIC opentelemetry_metrics)
36+
if(TARGET utf8_range::utf8_validity)
37+
target_link_libraries(
38+
opentelemetry_otlp_recordable
39+
PUBLIC opentelemetry_logs opentelemetry_metrics utf8_range::utf8_validity)
40+
target_compile_definitions(opentelemetry_otlp_recordable
41+
PRIVATE ENABLE_OTLP_UTF8_VALIDITY)
42+
else()
43+
target_link_libraries(opentelemetry_otlp_recordable
44+
PUBLIC opentelemetry_logs opentelemetry_metrics)
45+
endif()
3946

4047
#
4148
# opentelemetry_exporter_otlp_builder_utils
@@ -57,6 +64,12 @@ target_link_libraries(opentelemetry_exporter_otlp_builder_utils
5764
PUBLIC opentelemetry_otlp_recordable)
5865

5966
if(OPENTELEMETRY_INSTALL)
67+
if(TARGET opentelemetry_otlp_utf8_validity)
68+
opentelemetry_add_pkgconfig(
69+
otlp_utf8_validity "OpenTelemetry OTLP - UTF-8 Validity"
70+
"OTLP UTF-8 validity implementation for value conversion.")
71+
endif()
72+
6073
opentelemetry_add_pkgconfig(
6174
otlp_recordable
6275
"OpenTelemetry OTLP - Recordable"

‎exporters/otlp/src/otlp_populate_attribute_utils.cc‎

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
// Copyright The OpenTelemetry Authors
22
// SPDX-License-Identifier: Apache-2.0
33

4+
#if defined(ENABLE_OTLP_UTF8_VALIDITY)
5+
# include <utf8_validity.h>
6+
#endif
7+
48
#include <stdint.h>
59
#include <string>
610
#include <unordered_map>
@@ -81,12 +85,35 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
8185
}
8286
else if (nostd::holds_alternative<const char *>(value))
8387
{
84-
proto_value->set_string_value(nostd::get<const char *>(value));
88+
const char *str_value = nostd::get<const char *>(value);
89+
#if defined(ENABLE_OTLP_UTF8_VALIDITY)
90+
proto_value->set_string_value(str_value);
91+
#else
92+
if (utf8_range::IsStructurallyValid(str_value))
93+
{
94+
proto_value->set_string_value(str_value);
95+
}
96+
else
97+
{
98+
proto_value->set_bytes_value(str_value, strlen(str_value));
99+
}
100+
#endif
85101
}
86102
else if (nostd::holds_alternative<nostd::string_view>(value))
87103
{
88-
proto_value->set_string_value(nostd::get<nostd::string_view>(value).data(),
89-
nostd::get<nostd::string_view>(value).size());
104+
nostd::string_view str_value = nostd::get<nostd::string_view>(value);
105+
#if defined(ENABLE_OTLP_UTF8_VALIDITY)
106+
proto_value->set_string_value(str_value.data(), str_value.size());
107+
#else
108+
if (utf8_range::IsStructurallyValid({str_value.data(), str_value.size()}))
109+
{
110+
proto_value->set_string_value(str_value.data(), str_value.size());
111+
}
112+
else
113+
{
114+
proto_value->set_bytes_value(str_value.data(), str_value.size());
115+
}
116+
#endif
90117
}
91118
else if (nostd::holds_alternative<nostd::span<const uint8_t>>(value))
92119
{
@@ -159,7 +186,18 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
159186
auto array_value = proto_value->mutable_array_value();
160187
for (const auto &val : nostd::get<nostd::span<const nostd::string_view>>(value))
161188
{
189+
#if defined(ENABLE_OTLP_UTF8_VALIDITY)
162190
array_value->add_values()->set_string_value(val.data(), val.size());
191+
#else
192+
if (utf8_range::IsStructurallyValid({val.data(), val.size()}))
193+
{
194+
array_value->add_values()->set_string_value(val.data(), val.size());
195+
}
196+
else
197+
{
198+
array_value->add_values()->set_bytes_value(val.data(), val.size());
199+
}
200+
#endif
163201
}
164202
}
165203
}
@@ -224,7 +262,19 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
224262
}
225263
else if (nostd::holds_alternative<std::string>(value))
226264
{
227-
proto_value->set_string_value(nostd::get<std::string>(value));
265+
const std::string &str_value = nostd::get<std::string>(value);
266+
#if defined(ENABLE_OTLP_UTF8_VALIDITY)
267+
proto_value->set_string_value(str_value);
268+
#else
269+
if (utf8_range::IsStructurallyValid(str_value))
270+
{
271+
proto_value->set_string_value(str_value);
272+
}
273+
else
274+
{
275+
proto_value->set_bytes_value(str_value);
276+
}
277+
#endif
228278
}
229279
else if (nostd::holds_alternative<std::vector<bool>>(value))
230280
{
@@ -281,7 +331,18 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
281331
auto array_value = proto_value->mutable_array_value();
282332
for (const auto &val : nostd::get<std::vector<std::string>>(value))
283333
{
334+
#if defined(ENABLE_OTLP_UTF8_VALIDITY)
284335
array_value->add_values()->set_string_value(val);
336+
#else
337+
if (utf8_range::IsStructurallyValid(val))
338+
{
339+
array_value->add_values()->set_string_value(val);
340+
}
341+
else
342+
{
343+
array_value->add_values()->set_bytes_value(val);
344+
}
345+
#endif
285346
}
286347
}
287348
}

‎exporters/otlp/test/otlp_log_recordable_test.cc‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,15 @@ TEST(OtlpLogRecordable, Basic)
8484
memcmp(reinterpret_cast<const void *>(rec.log_record().body().bytes_value().data()),
8585
reinterpret_cast<const void *>(byte_arr), 5));
8686
EXPECT_EQ(rec.log_record().body().bytes_value().size(), 5);
87+
88+
// Test bytes body: non-UTF8 string_view is automatically converted to bytes_value
89+
const char byte_body[] = {'\x80', '\x81', '\x82', '\x83', '\x84'};
90+
nostd::string_view byte_body_view(byte_body, 5);
91+
rec.SetBody(byte_body_view);
92+
EXPECT_TRUE(0 ==
93+
memcmp(reinterpret_cast<const void *>(rec.log_record().body().bytes_value().data()),
94+
reinterpret_cast<const void *>(byte_body), 5));
95+
EXPECT_EQ(rec.log_record().body().bytes_value().size(), 5);
8796
}
8897

8998
TEST(OtlpLogRecordable, GetResource)
@@ -126,7 +135,12 @@ TEST(OtlpLogRecordable, SetSingleAttribute)
126135
uint8_t byte_arr[] = {'T', 'e', 's', 't'};
127136
common::AttributeValue byte_val(
128137
nostd::span<const uint8_t>{reinterpret_cast<const uint8_t *>(byte_arr), 4});
129-
rec.SetAttribute(byte_key, byte_val);
138+
139+
// Non-UTF8 string_view is automatically converted to bytes_value
140+
nostd::string_view non_utf8_string_key = "non_utf8_string_attr";
141+
const char non_utf8_string_arr[] = {'\x80', '\x81', '\x82', '\x83'};
142+
common::AttributeValue non_utf8_string_val(nostd::string_view(non_utf8_string_arr, 4));
143+
rec.SetAttribute(non_utf8_string_key, non_utf8_string_val);
130144

131145
int checked_attributes = 0;
132146
for (auto &attribute : rec.log_record().attributes())
@@ -154,8 +168,16 @@ TEST(OtlpLogRecordable, SetSingleAttribute)
154168
reinterpret_cast<const void *>(byte_arr), 4));
155169
EXPECT_EQ(attribute.value().bytes_value().size(), 4);
156170
}
171+
else if (attribute.key() == non_utf8_string_key)
172+
{
173+
++checked_attributes;
174+
EXPECT_TRUE(0 ==
175+
memcmp(reinterpret_cast<const void *>(attribute.value().bytes_value().data()),
176+
reinterpret_cast<const void *>(non_utf8_string_arr), 4));
177+
EXPECT_EQ(attribute.value().bytes_value().size(), 4);
178+
}
157179
}
158-
EXPECT_EQ(4, checked_attributes);
180+
EXPECT_EQ(5, checked_attributes);
159181
}
160182

161183
// Test non-int array types. Int array types are tested using templates (see IntAttributeTest)

0 commit comments

Comments
 (0)