Skip to content

[OTLP EXPORTER] fix OTLP recordable attribute spec compliance #4166

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.

Sign up for GitHub

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

Draft
dbarker wants to merge 3 commits into
base: main
Choose a base branch
from
+818 −247
Draft

[OTLP EXPORTER] fix OTLP recordable attribute spec compliance #4166

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1525b4c
ensure unique keys for otlp attributes, always allow bytes values in …
dbarker Jun 16, 2026
de16c15
fix ci issues
dbarker Jun 16, 2026
88e47f6
use unordered map to track otlp recordable attributes. check for null…
dbarker Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions exporters/otlp/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,20 @@ cc_library(
],
)

cc_test(
name = "otlp_populate_attribute_utils_test",
srcs = ["test/otlp_populate_attribute_utils_test.cc"],
tags = [
"otlp",
"test",
],
deps = [
":otlp_recordable",
"//sdk/src/metrics",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "otlp_recordable_test",
srcs = ["test/otlp_recordable_test.cc"],
Expand Down
15 changes: 15 additions & 0 deletions exporters/otlp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,21 @@ if(WITH_OTLP_FILE)
endif()

if(BUILD_TESTING)
add_executable(otlp_populate_attribute_utils_test
test/otlp_populate_attribute_utils_test.cc)
target_link_libraries(
otlp_populate_attribute_utils_test ${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT} opentelemetry_otlp_recordable
protobuf::libprotobuf)
gtest_add_tests(
TARGET otlp_populate_attribute_utils_test
TEST_PREFIX exporter.otlp.
TEST_LIST otlp_populate_attribute_utils_test)
if(WITH_OTLP_UTF8_VALIDITY AND TARGET utf8_range::utf8_validity)
target_compile_definitions(otlp_populate_attribute_utils_test
PRIVATE ENABLE_OTLP_UTF8_VALIDITY)
endif()

add_executable(otlp_recordable_test test/otlp_recordable_test.cc)
target_link_libraries(
otlp_recordable_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
Expand Down
20 changes: 9 additions & 11 deletions exporters/otlp/include/opentelemetry/exporters/otlp/otlp_populate_attribute_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,22 +56,20 @@ class OtlpPopulateAttributeUtils
&instrumentation_scope) noexcept;

static void PopulateAnyValue(opentelemetry::proto::common::v1::AnyValue *proto_value,
const opentelemetry::common::AttributeValue &value,
bool allow_bytes) noexcept;
const opentelemetry::common::AttributeValue &value) noexcept;

static void PopulateAnyValue(opentelemetry::proto::common::v1::AnyValue *proto_value,
const opentelemetry::sdk::common::OwnedAttributeValue &value,
bool allow_bytes) noexcept;
static void PopulateAnyValue(
opentelemetry::proto::common::v1::AnyValue *proto_value,
const opentelemetry::sdk::common::OwnedAttributeValue &value) noexcept;

static void PopulateAttribute(opentelemetry::proto::common::v1::KeyValue *attribute,
nostd::string_view key,
const opentelemetry::common::AttributeValue &value,
bool allow_bytes) noexcept;
const opentelemetry::common::AttributeValue &value) noexcept;

static void PopulateAttribute(opentelemetry::proto::common::v1::KeyValue *attribute,
nostd::string_view key,
const opentelemetry::sdk::common::OwnedAttributeValue &value,
bool allow_bytes) noexcept;
static void PopulateAttribute(
opentelemetry::proto::common::v1::KeyValue *attribute,
nostd::string_view key,
const opentelemetry::sdk::common::OwnedAttributeValue &value) noexcept;
};

} // namespace otlp
Expand Down
19 changes: 16 additions & 3 deletions exporters/otlp/src/otlp_log_recordable.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#include "opentelemetry/exporters/otlp/otlp_log_recordable.h"
#include <cstdint>
#include <string>

#include "opentelemetry/common/attribute_value.h"
#include "opentelemetry/common/timestamp.h"
#include "opentelemetry/exporters/otlp/otlp_log_recordable.h"
#include "opentelemetry/exporters/otlp/otlp_populate_attribute_utils.h"
#include "opentelemetry/logs/severity.h"
#include "opentelemetry/nostd/span.h"
Expand All @@ -18,6 +21,8 @@

// clang-format off
#include "opentelemetry/exporters/otlp/protobuf_include_prefix.h" // IWYU pragma: keep
#include <google/protobuf/repeated_ptr_field.h>
#include "opentelemetry/proto/common/v1/common.pb.h"
#include "opentelemetry/proto/logs/v1/logs.pb.h"
#include "opentelemetry/exporters/otlp/protobuf_include_suffix.h" // IWYU pragma: keep
// clang-format on
Expand Down Expand Up @@ -194,7 +199,7 @@ void OtlpLogRecordable::SetSeverity(opentelemetry::logs::Severity severity) noex

void OtlpLogRecordable::SetBody(const opentelemetry::common::AttributeValue &message) noexcept
{
OtlpPopulateAttributeUtils::PopulateAnyValue(proto_record_.mutable_body(), message, true);
OtlpPopulateAttributeUtils::PopulateAnyValue(proto_record_.mutable_body(), message);
}

void OtlpLogRecordable::SetEventId(int64_t /* id */, nostd::string_view event_name) noexcept
Expand Down Expand Up @@ -236,7 +241,15 @@ void OtlpLogRecordable::SetTraceFlags(const opentelemetry::trace::TraceFlags &tr
void OtlpLogRecordable::SetAttribute(opentelemetry::nostd::string_view key,
const opentelemetry::common::AttributeValue &value) noexcept
{
OtlpPopulateAttributeUtils::PopulateAttribute(proto_record_.add_attributes(), key, value, true);
for (auto &attr : *proto_record_.mutable_attributes())

@dbarker dbarker Jun 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Benchmark comparison:

This check to avoid duplicating attribute keys adds overhead:

  • ~2.3% (~14ns) overhead to nominal spans with 6 attributes (OtlpSpanFixture/Nominal)
  • ~136% (10us) to spans with max (128) attributes (OtlpSpanFixture/MaxAttributes)

This can be mitigated a few ways 1) add a SetAttributes method that takes key value iterable and does not check existing keys on creation of the span, 2) do not check attribute keys during span recording and perform deduplication when creating the protobuf request message on export.

branch: dbarker:add_otlp_recordable_benchmark (#4165)


-----------------------------------------------------------------------------------
Benchmark                                         Time             CPU   Iterations
-----------------------------------------------------------------------------------
OtlpSpanFixture/Minimal_mean                    342 ns          345 ns            5
OtlpSpanFixture/Minimal_median                  343 ns          345 ns            5
OtlpSpanFixture/Minimal_stddev                 2.23 ns         1.88 ns            5
OtlpSpanFixture/Minimal_cv                     0.65 %          0.55 %             5
OtlpSpanFixture/Nominal_mean                    596 ns          598 ns            5
OtlpSpanFixture/Nominal_median                  594 ns          596 ns            5
OtlpSpanFixture/Nominal_stddev                 10.5 ns         10.6 ns            5
OtlpSpanFixture/Nominal_cv                     1.76 %          1.77 %             5
OtlpSpanFixture/MaxAttributes_mean             7517 ns         7521 ns            5 items_per_second=17.0197M/s
OtlpSpanFixture/MaxAttributes_median           7489 ns         7492 ns            5 items_per_second=17.0841M/s
OtlpSpanFixture/MaxAttributes_stddev           87.8 ns         87.8 ns            5 items_per_second=196.123k/s
OtlpSpanFixture/MaxAttributes_cv               1.17 %          1.17 %             5 items_per_second=1.15%
BM_PopulateRequest/span_count:1_mean          0.378 us        0.378 us            5 items_per_second=2.6486M/s
BM_PopulateRequest/span_count:1_median        0.378 us        0.378 us            5 items_per_second=2.64307M/s
BM_PopulateRequest/span_count:1_stddev        0.002 us        0.002 us            5 items_per_second=11.8787k/s
BM_PopulateRequest/span_count:1_cv             0.45 %          0.45 %             5 items_per_second=0.45%
BM_PopulateRequest/span_count:512_mean         25.5 us         25.5 us            5 items_per_second=20.0764M/s
BM_PopulateRequest/span_count:512_median       25.4 us         25.4 us            5 items_per_second=20.1812M/s
BM_PopulateRequest/span_count:512_stddev      0.248 us        0.248 us            5 items_per_second=194k/s
BM_PopulateRequest/span_count:512_cv           0.97 %          0.97 %             5 items_per_second=0.97%

branch: dbarker:fix_otlp_attribute_spec_compliance (this PR #4166)

-----------------------------------------------------------------------------------
Benchmark                                         Time             CPU   Iterations
-----------------------------------------------------------------------------------
OtlpSpanFixture/Minimal_mean                    340 ns          343 ns            5
OtlpSpanFixture/Minimal_median                  340 ns          344 ns            5
OtlpSpanFixture/Minimal_stddev                0.711 ns        0.795 ns            5
OtlpSpanFixture/Minimal_cv                     0.21 %          0.23 %             5
OtlpSpanFixture/Nominal_mean                    610 ns          614 ns            5
OtlpSpanFixture/Nominal_median                  607 ns          611 ns            5
OtlpSpanFixture/Nominal_stddev                 6.20 ns         6.58 ns            5
OtlpSpanFixture/Nominal_cv                     1.02 %          1.07 %             5
OtlpSpanFixture/MaxAttributes_mean            17773 ns        17776 ns            5 items_per_second=7.20295M/s
OtlpSpanFixture/MaxAttributes_median          17727 ns        17730 ns            5 items_per_second=7.21921M/s
OtlpSpanFixture/MaxAttributes_stddev            337 ns          336 ns            5 items_per_second=134.931k/s
OtlpSpanFixture/MaxAttributes_cv               1.89 %          1.89 %             5 items_per_second=1.87%
BM_PopulateRequest/span_count:1_mean          0.349 us        0.349 us            5 items_per_second=2.86195M/s
BM_PopulateRequest/span_count:1_median        0.349 us        0.349 us            5 items_per_second=2.86567M/s
BM_PopulateRequest/span_count:1_stddev        0.001 us        0.001 us            5 items_per_second=10.7304k/s
BM_PopulateRequest/span_count:1_cv             0.37 %          0.38 %             5 items_per_second=0.37%
BM_PopulateRequest/span_count:512_mean         26.1 us         26.1 us            5 items_per_second=19.5965M/s
BM_PopulateRequest/span_count:512_median       26.1 us         26.1 us            5 items_per_second=19.5943M/s
BM_PopulateRequest/span_count:512_stddev      0.104 us        0.104 us            5 items_per_second=77.6371k/s
BM_PopulateRequest/span_count:512_cv           0.40 %          0.40 %             5 items_per_second=0.40%

{
if (attr.key() == key)

@owent owent Jun 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we use a hash map (e.g., unordered_map) to track existing keys? The current linear scan on every SetAttribute call is O(n) per insertion—O(n²) total—and the overhead compounds quickly at scale.

It's not just the number of attributes: string comparison cost grows with key length. Each loop iteration does a full character-by-character comparison, whereas a hash map typically checks hash codes first, only falling back to full comparison on collisions.

@dbarker dbarker Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

88e47f6

This implements the hash map for attribute keys.

Compared to the linear search (1525b4c), the performance is better in the max attributes case by -5.5us or -31%, but worse for the nominal attribute case (6 attributes) by ~220 ns or +37%.

Benchmark main linear search (1525b4c) hash map (88e47f6)
Minimal 342 ns 340 ns 359 ns
Nominal 596 ns 610 ns 835 ns
MaxAttributes 7,517 ns 17,773 ns 12,256 ns
PopulateRequest/1 378 ns 349 ns 367 ns
PopulateRequest/512 25.5 µs 26.1 µs 26.0 µs 
-----------------------------------------------------------------------------------
Benchmark                                         Time             CPU   Iterations
-----------------------------------------------------------------------------------
OtlpSpanFixture/Minimal_mean                    359 ns          361 ns            5
OtlpSpanFixture/Minimal_median                  359 ns          361 ns            5
OtlpSpanFixture/Minimal_stddev                0.440 ns        0.477 ns            5
OtlpSpanFixture/Minimal_cv                     0.12 %          0.13 %             5
OtlpSpanFixture/Nominal_mean                    835 ns          839 ns            5
OtlpSpanFixture/Nominal_median                  834 ns          839 ns            5
OtlpSpanFixture/Nominal_stddev                 5.53 ns         5.61 ns            5
OtlpSpanFixture/Nominal_cv                     0.66 %          0.67 %             5
OtlpSpanFixture/MaxAttributes_mean            12256 ns        12261 ns            5 items_per_second=10.4397M/s
OtlpSpanFixture/MaxAttributes_median          12233 ns        12238 ns            5 items_per_second=10.4593M/s
OtlpSpanFixture/MaxAttributes_stddev           62.0 ns         61.9 ns            5 items_per_second=52.5819k/s
OtlpSpanFixture/MaxAttributes_cv               0.51 %          0.50 %             5 items_per_second=0.50%
BM_PopulateRequest/span_count:1_mean          0.367 us        0.367 us            5 items_per_second=2.72565M/s
BM_PopulateRequest/span_count:1_median        0.367 us        0.367 us            5 items_per_second=2.72547M/s
BM_PopulateRequest/span_count:1_stddev        0.001 us        0.001 us            5 items_per_second=4.4662k/s
BM_PopulateRequest/span_count:1_cv             0.16 %          0.16 %             5 items_per_second=0.16%
BM_PopulateRequest/span_count:512_mean         26.0 us         26.0 us            5 items_per_second=19.6842M/s
BM_PopulateRequest/span_count:512_median       26.0 us         26.0 us            5 items_per_second=19.7045M/s
BM_PopulateRequest/span_count:512_stddev      0.212 us        0.212 us            5 items_per_second=160.233k/s
BM_PopulateRequest/span_count:512_cv           0.82 %          0.82 %             5 items_per_second=0.81%

{
OtlpPopulateAttributeUtils::PopulateAttribute(&attr, key, value);
return;
}
}
OtlpPopulateAttributeUtils::PopulateAttribute(proto_record_.add_attributes(), key, value);
}

void OtlpLogRecordable::SetResource(const opentelemetry::sdk::resource::Resource &resource) noexcept
Expand Down
8 changes: 4 additions & 4 deletions exporters/otlp/src/otlp_metric_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ void OtlpMetricUtils::ConvertSumMetric(const metric_sdk::MetricData &metric_data
for (auto &kv_attr : point_data_with_attributes.attributes)
{
OtlpPopulateAttributeUtils::PopulateAttribute(proto_sum_point_data->add_attributes(),
kv_attr.first, kv_attr.second, false);
kv_attr.first, kv_attr.second);
}
}
}
Expand Down Expand Up @@ -178,7 +178,7 @@ void OtlpMetricUtils::ConvertHistogramMetric(
for (auto &kv_attr : point_data_with_attributes.attributes)
{
OtlpPopulateAttributeUtils::PopulateAttribute(proto_histogram_point_data->add_attributes(),
kv_attr.first, kv_attr.second, false);
kv_attr.first, kv_attr.second);
}
}
}
Expand Down Expand Up @@ -242,7 +242,7 @@ void OtlpMetricUtils::ConvertExponentialHistogramMetric(
for (auto &kv_attr : point_data_with_attributes.attributes)
{
OtlpPopulateAttributeUtils::PopulateAttribute(proto_histogram_point_data->add_attributes(),
kv_attr.first, kv_attr.second, false);
kv_attr.first, kv_attr.second);
}
}
}
Expand Down Expand Up @@ -272,7 +272,7 @@ void OtlpMetricUtils::ConvertGaugeMetric(const opentelemetry::sdk::metrics::Metr
for (auto &kv_attr : point_data_with_attributes.attributes)
{
OtlpPopulateAttributeUtils::PopulateAttribute(proto_gauge_point_data->add_attributes(),
kv_attr.first, kv_attr.second, false);
kv_attr.first, kv_attr.second);
}
}
}
Expand Down
Loading
Loading