Skip to content

Commit d800b20

Browse files
owentowent
committed
Add unit tests for resources and trace span
1 parent d89343c commit d800b20

File tree

7 files changed

+206
-20
lines changed

7 files changed

+206
-20
lines changed

‎exporters/otlp/src/otlp_populate_attribute_utils.cc

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "opentelemetry/nostd/string_view.h"
1414
#include "opentelemetry/nostd/variant.h"
1515
#include "opentelemetry/sdk/common/attribute_utils.h"
16+
#include "opentelemetry/sdk/common/attribute_validity.h"
1617
#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h"
1718
#include "opentelemetry/sdk/resource/resource.h"
1819
#include "opentelemetry/version.h"
@@ -85,8 +86,18 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
8586
}
8687
else if (nostd::holds_alternative<nostd::string_view>(value))
8788
{
88-
proto_value->set_string_value(nostd::get<nostd::string_view>(value).data(),
89-
nostd::get<nostd::string_view>(value).size());
89+
if (allow_bytes &&
90+
!opentelemetry::sdk::common::AttributeIsValidString(nostd::get<nostd::string_view>(value)))
91+
{
92+
proto_value->set_bytes_value(
93+
reinterpret_cast<const void *>(nostd::get<nostd::string_view>(value).data()),
94+
nostd::get<nostd::string_view>(value).size());
95+
}
96+
else
97+
{
98+
proto_value->set_string_value(nostd::get<nostd::string_view>(value).data(),
99+
nostd::get<nostd::string_view>(value).size());
100+
}
90101
}
91102
else if (nostd::holds_alternative<nostd::span<const uint8_t>>(value))
92103
{
@@ -159,7 +170,15 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
159170
auto array_value = proto_value->mutable_array_value();
160171
for (const auto &val : nostd::get<nostd::span<const nostd::string_view>>(value))
161172
{
162-
array_value->add_values()->set_string_value(val.data(), val.size());
173+
if (allow_bytes && !opentelemetry::sdk::common::AttributeIsValidString(val))
174+
{
175+
array_value->add_values()->set_bytes_value(reinterpret_cast<const void *>(val.data()),
176+
val.size());
177+
}
178+
else
179+
{
180+
array_value->add_values()->set_string_value(val.data(), val.size());
181+
}
163182
}
164183
}
165184
}
@@ -224,7 +243,17 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
224243
}
225244
else if (nostd::holds_alternative<std::string>(value))
226245
{
227-
proto_value->set_string_value(nostd::get<std::string>(value));
246+
if (allow_bytes &&
247+
!opentelemetry::sdk::common::AttributeIsValidString(nostd::get<std::string>(value)))
248+
{
249+
proto_value->set_bytes_value(
250+
reinterpret_cast<const void *>(nostd::get<std::string>(value).data()),
251+
nostd::get<std::string>(value).size());
252+
}
253+
else
254+
{
255+
proto_value->set_string_value(nostd::get<std::string>(value));
256+
}
228257
}
229258
else if (nostd::holds_alternative<std::vector<bool>>(value))
230259
{
@@ -281,7 +310,15 @@ void OtlpPopulateAttributeUtils::PopulateAnyValue(
281310
auto array_value = proto_value->mutable_array_value();
282311
for (const auto &val : nostd::get<std::vector<std::string>>(value))
283312
{
284-
array_value->add_values()->set_string_value(val);
313+
if (allow_bytes && !opentelemetry::sdk::common::AttributeIsValidString(val))
314+
{
315+
array_value->add_values()->set_bytes_value(reinterpret_cast<const void *>(val.data()),
316+
val.size());
317+
}
318+
else
319+
{
320+
array_value->add_values()->set_string_value(val);
321+
}
285322
}
286323
}
287324
}

‎sdk/include/opentelemetry/sdk/common/attribute_validity.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ struct AttributeValidator
7575
bool operator()(const std::vector<uint64_t> & /*v*/) noexcept { return true; }
7676
bool operator()(const std::vector<uint8_t> & /*v*/) noexcept { return true; }
7777

78+
OPENTELEMETRY_EXPORT static bool IsValid(const std::string &value) noexcept;
79+
80+
OPENTELEMETRY_EXPORT static bool IsValid(nostd::string_view value) noexcept;
81+
7882
OPENTELEMETRY_EXPORT static bool IsValid(const OwnedAttributeValue &value) noexcept;
7983

8084
OPENTELEMETRY_EXPORT static bool IsValid(

‎sdk/src/common/attribute_validity.cc

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,16 @@ OPENTELEMETRY_EXPORT bool AttributeIsValidString(nostd::string_view value) noexc
3434
return 0 != utf8_range::utf8_range_IsValid(value.data(), value.size());
3535
}
3636

37+
OPENTELEMETRY_EXPORT bool AttributeValidator::IsValid(const std::string &value) noexcept
38+
{
39+
return AttributeIsValidString(value);
40+
}
41+
42+
OPENTELEMETRY_EXPORT bool AttributeValidator::IsValid(nostd::string_view value) noexcept
43+
{
44+
return AttributeIsValidString(value);
45+
}
46+
3747
OPENTELEMETRY_EXPORT bool AttributeValidator::IsValid(const OwnedAttributeValue &value) noexcept
3848
{
3949
#if OPENTELEMETRY_HAVE_EXCEPTIONS
@@ -102,9 +112,18 @@ OPENTELEMETRY_EXPORT void AttributeValidator::Filter(AttributeMap &attributes,
102112
std::unordered_set<std::string> invalid_keys;
103113
for (auto &kv : attributes)
104114
{
115+
if (!common::AttributeValidator::IsValid(kv.first))
116+
{
117+
OTEL_INTERNAL_LOG_WARN(log_hint << " Invalid attribute key " << kv.first
118+
<< ". This attribute will be ignored.");
119+
120+
invalid_keys.insert(kv.first);
121+
continue;
122+
}
123+
105124
if (!common::AttributeValidator::IsValid(kv.second))
106125
{
107-
OTEL_INTERNAL_LOG_WARN(log_hint << " Invalid attribute value for: " << kv.first
126+
OTEL_INTERNAL_LOG_WARN(log_hint << " Invalid attribute value for " << kv.first
108127
<< ". This attribute will be ignored.");
109128

110129
invalid_keys.insert(kv.first);
@@ -123,9 +142,18 @@ OPENTELEMETRY_EXPORT void AttributeValidator::Filter(OrderedAttributeMap &attrib
123142
std::unordered_set<std::string> invalid_keys;
124143
for (auto &kv : attributes)
125144
{
145+
if (!common::AttributeValidator::IsValid(kv.first))
146+
{
147+
OTEL_INTERNAL_LOG_WARN(log_hint << " Invalid attribute key " << kv.first
148+
<< ". This attribute will be ignored.");
149+
150+
invalid_keys.insert(kv.first);
151+
continue;
152+
}
153+
126154
if (!common::AttributeValidator::IsValid(kv.second))
127155
{
128-
OTEL_INTERNAL_LOG_WARN(log_hint << " Invalid attribute value for: " << kv.first
156+
OTEL_INTERNAL_LOG_WARN(log_hint << " Invalid attribute value for " << kv.first
129157
<< ". This attribute will be ignored.");
130158

131159
invalid_keys.insert(kv.first);
@@ -155,7 +183,7 @@ bool KeyValueFilterIterable::ForEachKeyValue(
155183
bool ret =
156184
origin_->ForEachKeyValue([&size, &callback, this](opentelemetry::nostd::string_view k,
157185
opentelemetry::common::AttributeValue v) {
158-
if (AttributeValidator::IsValid(v))
186+
if (AttributeValidator::IsValid(k) && AttributeValidator::IsValid(v))
159187
{
160188
++size;
161189
return callback(k, v);
@@ -184,8 +212,8 @@ size_t KeyValueFilterIterable::size() const noexcept
184212

185213
size_t size = 0;
186214
origin_->ForEachKeyValue(
187-
[&size](opentelemetry::nostd::string_view, opentelemetry::common::AttributeValue v) {
188-
if (AttributeValidator::IsValid(v))
215+
[&size](opentelemetry::nostd::string_view k, opentelemetry::common::AttributeValue v) {
216+
if (AttributeValidator::IsValid(k) && AttributeValidator::IsValid(v))
189217
{
190218
++size;
191219
}

‎sdk/src/resource/resource.cc

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,16 @@ Resource::Resource(const ResourceAttributes &attributes, const std::string &sche
2929
attributes_.reserve(attributes.size());
3030
for (auto &kv : attributes)
3131
{
32+
if (!common::AttributeValidator::IsValid(kv.first))
33+
{
34+
OTEL_INTERNAL_LOG_WARN("[Resource] Invalid attribute key "
35+
<< kv.first << ". This attribute will be ignored.");
36+
continue;
37+
}
38+
3239
if (!common::AttributeValidator::IsValid(kv.second))
3340
{
34-
OTEL_INTERNAL_LOG_WARN("[Resource] Invalid attribute value for: "
41+
OTEL_INTERNAL_LOG_WARN("[Resource] Invalid attribute value for "
3542
<< kv.first << ". This attribute will be ignored.");
3643
continue;
3744
}

‎sdk/src/trace/span.cc

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,17 @@ Span::Span(std::shared_ptr<Tracer> &&tracer,
7878

7979
recordable_->SetTraceFlags(span_context_->trace_flags());
8080

81-
attributes.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept {
82-
recordable_->SetAttribute(key, value);
83-
return true;
84-
});
81+
opentelemetry::sdk::common::KeyValueFilterIterable attributes_filter(attributes, "[Trace Span] ");
82+
attributes_filter.ForEachKeyValue(
83+
[&](nostd::string_view key, common::AttributeValue value) noexcept {
84+
recordable_->SetAttribute(key, value);
85+
return true;
86+
});
8587

8688
links.ForEachKeyValue([&](const opentelemetry::trace::SpanContext &span_context,
8789
const common::KeyValueIterable &attributes) {
88-
recordable_->AddLink(span_context, attributes);
90+
recordable_->AddLink(span_context, opentelemetry::sdk::common::KeyValueFilterIterable(
91+
attributes, "[Trace Span Link] "));
8992
return true;
9093
});
9194

@@ -109,9 +112,16 @@ void Span::SetAttribute(nostd::string_view key, const common::AttributeValue &va
109112
return;
110113
}
111114

115+
if (!sdk::common::AttributeValidator::IsValid(key))
116+
{
117+
OTEL_INTERNAL_LOG_WARN("[Trace Span] Invalid span attribute key "
118+
<< key << ". This attribute will be ignored.");
119+
return;
120+
}
121+
112122
if (!sdk::common::AttributeValidator::IsValid(value))
113123
{
114-
OTEL_INTERNAL_LOG_WARN("[Trace Span] Invalid span attribute value for: "
124+
OTEL_INTERNAL_LOG_WARN("[Trace Span] Invalid span attribute value for "
115125
<< key << ". This attribute will be ignored.");
116126
return;
117127
}
@@ -146,7 +156,8 @@ void Span::AddEvent(nostd::string_view name, const common::KeyValueIterable &att
146156
{
147157
return;
148158
}
149-
recordable_->AddEvent(name, attributes);
159+
recordable_->AddEvent(
160+
name, opentelemetry::sdk::common::KeyValueFilterIterable(attributes, "[Trace Span Event] "));
150161
}
151162

152163
void Span::AddEvent(nostd::string_view name,
@@ -158,7 +169,9 @@ void Span::AddEvent(nostd::string_view name,
158169
{
159170
return;
160171
}
161-
recordable_->AddEvent(name, timestamp, attributes);
172+
recordable_->AddEvent(
173+
name, timestamp,
174+
opentelemetry::sdk::common::KeyValueFilterIterable(attributes, "[Trace Span Event] "));
162175
}
163176

164177
#if OPENTELEMETRY_ABI_VERSION_NO >= 2
@@ -185,7 +198,8 @@ void Span::AddLinks(const opentelemetry::trace::SpanContextKeyValueIterable &lin
185198

186199
links.ForEachKeyValue([&](const opentelemetry::trace::SpanContext &span_context,
187200
const common::KeyValueIterable &attributes) {
188-
recordable_->AddLink(span_context, attributes);
201+
recordable_->AddLink(span_context, opentelemetry::sdk::common::KeyValueFilterIterable(
202+
attributes, "[Trace Span Link] "));
189203
return true;
190204
});
191205
}

‎sdk/test/resource/resource_test.cc

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,36 @@ TEST(ResourceTest, create_with_schemaurl)
145145
EXPECT_EQ(received_schema_url, schema_url);
146146
}
147147

148+
TEST(ResourceTest, create_with_invalid_attributes)
149+
{
150+
ResourceAttributes expected_attributes = {
151+
{semconv::telemetry::kTelemetrySdkLanguage, "cpp"},
152+
{semconv::telemetry::kTelemetrySdkName, "opentelemetry"},
153+
{semconv::telemetry::kTelemetrySdkVersion, OPENTELEMETRY_SDK_VERSION},
154+
{semconv::service::kServiceName, "unknown_service"},
155+
};
156+
ResourceAttributes attributes = {
157+
{semconv::telemetry::kTelemetrySdkLanguage, "cpp"},
158+
{semconv::telemetry::kTelemetrySdkName, "opentelemetry"},
159+
{semconv::telemetry::kTelemetrySdkVersion, OPENTELEMETRY_SDK_VERSION},
160+
{semconv::service::kServiceName, "unknown_service"},
161+
{"invalid_key\xff", "valid_value"},
162+
{"valid_key", "invalid_value\xff"},
163+
};
164+
auto resource = Resource::Create(attributes);
165+
auto received_attributes = resource.GetAttributes();
166+
for (auto &e : received_attributes)
167+
{
168+
EXPECT_TRUE(expected_attributes.find(e.first) != expected_attributes.end());
169+
if (expected_attributes.find(e.first) != expected_attributes.end())
170+
{
171+
EXPECT_EQ(opentelemetry::nostd::get<std::string>(expected_attributes.find(e.first)->second),
172+
opentelemetry::nostd::get<std::string>(e.second));
173+
}
174+
}
175+
EXPECT_EQ(received_attributes.size(), expected_attributes.size());
176+
}
177+
148178
TEST(ResourceTest, Merge)
149179
{
150180
TestResource resource1(ResourceAttributes({{"service", "backend"}}));

‎sdk/test/trace/tracer_test.cc

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,27 @@ TEST(Tracer, StartSpanWithAttributesCopy)
398398
ASSERT_EQ("c", strings[2]);
399399
}
400400

401+
TEST(Tracer, StartSpanWithInvalidAttributes)
402+
{
403+
InMemorySpanExporter *exporter = new InMemorySpanExporter();
404+
std::shared_ptr<InMemorySpanData> span_data = exporter->GetData();
405+
auto tracer = initTracer(std::unique_ptr<SpanExporter>{exporter});
406+
407+
{
408+
tracer
409+
->StartSpan("span 1",
410+
{
411+
{"attr1", "value1"},
412+
{"invalid_key\xff", "valid_value"},
413+
{"valid_key", "invalid_value\xff"},
414+
})
415+
->End();
416+
}
417+
418+
auto spans = span_data->GetSpans();
419+
ASSERT_EQ(1, spans.size());
420+
}
421+
401422
TEST(Tracer, GetSampler)
402423
{
403424
auto resource = Resource::Create({});
@@ -586,6 +607,30 @@ TEST(Tracer, StartSpanWithCustomConfig)
586607
#endif
587608
}
588609

610+
TEST(Tracer, SpanSetEventsWithInvalidAttributes)
611+
{
612+
InMemorySpanExporter *exporter = new InMemorySpanExporter();
613+
std::shared_ptr<InMemorySpanData> span_data = exporter->GetData();
614+
auto tracer = initTracer(std::unique_ptr<SpanExporter>{exporter});
615+
616+
auto span = tracer->StartSpan("span 1");
617+
span->AddEvent("event 3", std::chrono::system_clock::now(),
618+
{
619+
{"attr1", 1},
620+
{"invalid_key\xff", "valid_value"},
621+
{"valid_key", "invalid_value\xff"},
622+
});
623+
span->End();
624+
625+
auto spans = span_data->GetSpans();
626+
ASSERT_EQ(1, spans.size());
627+
628+
auto &span_data_events = spans.at(0)->GetEvents();
629+
ASSERT_EQ(1, span_data_events.size());
630+
ASSERT_EQ("event 3", span_data_events[0].GetName());
631+
ASSERT_EQ(1, span_data_events[0].GetAttributes().size());
632+
}
633+
589634
TEST(Tracer, StartSpanWithCustomConfigDifferingConditionOrder)
590635
{
591636
std::shared_ptr<opentelemetry::trace::Tracer> noop_tracer =
@@ -707,6 +752,27 @@ TEST(Tracer, SpanSetLinks)
707752
ASSERT_EQ(nostd::get<std::string>(link2.GetAttributes().at("attr3")), "3");
708753
ASSERT_EQ(nostd::get<std::string>(link2.GetAttributes().at("attr4")), "4");
709754
}
755+
756+
{
757+
758+
// Span link with invalid attributes
759+
tracer
760+
->StartSpan("efg", {{"attr1", 1}},
761+
{{SpanContext(false, false),
762+
{
763+
{"attr2", 2},
764+
{"invalid_key\xff", "valid_value"},
765+
{"valid_key", "invalid_value\xff"},
766+
}}})
767+
->End();
768+
auto spans = span_data->GetSpans();
769+
ASSERT_EQ(1, spans.size());
770+
771+
auto &span_data_links = spans.at(0)->GetLinks();
772+
ASSERT_EQ(1, span_data_links.size());
773+
auto link = span_data_links.at(0);
774+
ASSERT_EQ(nostd::get<int>(link.GetAttributes().at("attr2")), 2);
775+
}
710776
}
711777

712778
#if OPENTELEMETRY_ABI_VERSION_NO >= 2

0 commit comments

Comments
 (0)