Skip to content

Commit

Permalink
[API] Propagation: fix for hex conversion to binary for odd hex strin…
Browse files Browse the repository at this point in the history
…gs (#2533)
  • Loading branch information
karusher authored Feb 14, 2024
1 parent 614b8db commit c7a88c4
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 5 deletions.
12 changes: 7 additions & 5 deletions api/include/opentelemetry/trace/propagation/detail/hex.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,17 @@ inline bool HexToBinary(nostd::string_view hex, uint8_t *buffer, size_t buffer_s
int64_t buffer_pos = int64_t(buffer_size) - (hex_size + 1) / 2;
int64_t last_hex_pos = hex_size - 1;

int64_t i = 0;
for (; i < last_hex_pos; i += 2)
bool is_hex_size_odd = (hex_size % 2) == 1;
int64_t i = 0;

if (is_hex_size_odd)
{
buffer[buffer_pos++] = (HexToInt(hex[i]) << 4) | HexToInt(hex[i + 1]);
buffer[buffer_pos++] = HexToInt(hex[i++]);
}

if (i == last_hex_pos)
for (; i < last_hex_pos; i += 2)
{
buffer[buffer_pos] = HexToInt(hex[i]);
buffer[buffer_pos++] = (HexToInt(hex[i]) << 4) | HexToInt(hex[i + 1]);
}

return true;
Expand Down
2 changes: 2 additions & 0 deletions api/test/trace/propagation/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Copyright The OpenTelemetry Authors
# SPDX-License-Identifier: Apache-2.0

add_subdirectory(detail)

foreach(testname http_text_format_test b3_propagation_test)
add_executable(${testname} "${testname}.cc")
target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES}
Expand Down
20 changes: 20 additions & 0 deletions api/test/trace/propagation/detail/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Copyright The OpenTelemetry Authors
# SPDX-License-Identifier: Apache-2.0

load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark")

cc_test(
name = "hex_test",
srcs = [
"hex_test.cc",
],
tags = [
"api",
"test",
"trace",
],
deps = [
"//api",
"@com_google_googletest//:gtest_main",
],
)
12 changes: 12 additions & 0 deletions api/test/trace/propagation/detail/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright The OpenTelemetry Authors
# SPDX-License-Identifier: Apache-2.0

foreach(testname hex_test)
add_executable(${testname} "${testname}.cc")
target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT} opentelemetry_api)
gtest_add_tests(
TARGET ${testname}
TEST_PREFIX trace.
TEST_LIST ${testname})
endforeach()
42 changes: 42 additions & 0 deletions api/test/trace/propagation/detail/hex_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#include "opentelemetry/trace/propagation/detail/hex.h"

#include <map>

#include <gtest/gtest.h>

using namespace opentelemetry;

TEST(HexTest, ConvertOddLength)
{
const int kLength = 16;
std::string trace_id_hex = "78cfcfec62ae9e9";
uint8_t trace_id[kLength];
trace::propagation::detail::HexToBinary(trace_id_hex, trace_id, sizeof(trace_id));

const uint8_t expected_trace_id[kLength] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x7, 0x8c, 0xfc, 0xfe, 0xc6, 0x2a, 0xe9, 0xe9};

for (int i = 0; i < kLength; ++i)
{
EXPECT_EQ(trace_id[i], expected_trace_id[i]);
}
}

TEST(HexTest, ConvertEvenLength)
{
const int kLength = 16;
std::string trace_id_hex = "078cfcfec62ae9e9";
uint8_t trace_id[kLength];
trace::propagation::detail::HexToBinary(trace_id_hex, trace_id, sizeof(trace_id));

const uint8_t expected_trace_id[kLength] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x7, 0x8c, 0xfc, 0xfe, 0xc6, 0x2a, 0xe9, 0xe9};

for (int i = 0; i < kLength; ++i)
{
EXPECT_EQ(trace_id[i], expected_trace_id[i]);
}
}

1 comment on commit c7a88c4

@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: c7a88c4 Previous: 614b8db Ratio
BM_BaselineBuffer/2 7307143.211364746 ns/iter 2995665.244385096 ns/iter 2.44
BM_BaselineBuffer/4 8328287.601470947 ns/iter 2513576.860297216 ns/iter 3.31
BM_LockFreeBuffer/2 3124939.441680908 ns/iter 975008.3350298698 ns/iter 3.21
BM_LockFreeBuffer/4 10638144.01626587 ns/iter 1110630.0977320452 ns/iter 9.58

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

Please sign in to comment.