Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 8 additions & 2 deletions src/records/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,14 @@ target_link_libraries(

if(BUILD_TESTING)
add_executable(
test_records unit_tests/unit_test_main.cc unit_tests/test_RecHttp.cc unit_tests/test_RecUtils.cc
unit_tests/test_RecRegister.cc unit_tests/test_ConfigReloadTask.cc unit_tests/test_ConfigRegistry.cc
test_records
unit_tests/unit_test_main.cc
unit_tests/test_RecHttp.cc
unit_tests/test_RecUtils.cc
unit_tests/test_RecRegister.cc
unit_tests/test_ConfigReloadTask.cc
unit_tests/test_ConfigRegistry.cc
unit_tests/test_RecDumpRecords.cc
)
target_link_libraries(test_records PRIVATE records configmanager inkevent Catch2::Catch2 ts::tscore libswoc::libswoc)
add_catch2_test(NAME test_records COMMAND test_records)
Expand Down
7 changes: 7 additions & 0 deletions src/records/RecCore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "records/RecDefs.h"
#include "swoc/swoc_file.h"

#include "ts/apidefs.h"
#include "tscore/ink_platform.h"
#include "tscore/ink_memory.h"
#include "tscore/ink_string.h"
Expand Down Expand Up @@ -940,6 +941,12 @@ RecDumpRecords(RecT rec_type, RecDumpEntryCb callback, void *edata)
callback(RECT_PLUGIN, edata, true, name.data(),
type == Metrics::MetricType::COUNTER ? TS_RECORDDATATYPE_COUNTER : TS_RECORDDATATYPE_INT, &datum);
}

ts::Metrics::StaticString::instance().for_each([&](const std::string &name, const std::string &value) {
datum.rec_string = const_cast<char *>(value.c_str());

callback(RECT_PLUGIN, edata, true, name.data(), TS_RECORDDATATYPE_STRING, &datum);
});
Comment on lines +945 to +949
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

ts::Metrics::StaticString::for_each() invokes its callback while holding the internal mutex (per Metrics.h). Calling RecDumpRecords()'s external callback(...) under that lock risks deadlocks / long lock hold times if the callback does anything non-trivial. Consider snapshotting the (name,value) pairs while holding the mutex, then invoking callback(...) after releasing it.

Copilot uses AI. Check for mistakes.
}

void
Expand Down
81 changes: 81 additions & 0 deletions src/records/unit_tests/test_RecDumpRecords.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/** @file

Catch-based tests for RecDumpRecords

@section license License

Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements.
See the NOTICE file distributed with this work for additional information regarding copyright
ownership. The ASF licenses this file to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance with the License. You may obtain a
copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under the License
is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
or implied. See the License for the specific language governing permissions and limitations under
the License.
*/
#include <catch2/catch_test_macros.hpp>
#include <string>
#include <vector>

#include "../P_RecCore.h"
#include "tsutil/Metrics.h"

struct DumpEntry {
RecT rec_type;
int registered;
std::string name;
int data_type;
std::string string_value;
RecInt int_value{0};
};

static void
collect_callback(RecT rec_type, void *edata, int registered, const char *name, int data_type, RecData *datum)
{
auto *entries = static_cast<std::vector<DumpEntry> *>(edata);
DumpEntry entry;

entry.rec_type = rec_type;
entry.registered = registered;
entry.name = name;
entry.data_type = data_type;

if (data_type == RECD_STRING && datum->rec_string != nullptr) {
entry.string_value = datum->rec_string;
} else if (data_type == RECD_INT || data_type == RECD_COUNTER) {
entry.int_value = datum->rec_int;
Comment on lines +49 to +50
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

collect_callback() reads datum->rec_int for RECD_COUNTER entries. Since RecData is a union, counters should be read from datum->rec_counter when data_type == RECD_COUNTER (reading a non-active union member is undefined behavior and can trip UBSan even if the value isn't later asserted in this test).

Suggested change
} else if (data_type == RECD_INT || data_type == RECD_COUNTER) {
entry.int_value = datum->rec_int;
} else if (data_type == RECD_INT) {
entry.int_value = datum->rec_int;
} else if (data_type == RECD_COUNTER) {
entry.int_value = datum->rec_counter;

Copilot uses AI. Check for mistakes.
}

entries->push_back(std::move(entry));
}

TEST_CASE("RecDumpRecords - StaticString metrics", "[librecords][RecDump]")
{
const std::string test_name = "proxy.test.dump.string_metric";
const std::string test_value = "test_string_value";

ts::Metrics::StaticString::createString(test_name, test_value);

std::vector<DumpEntry> entries;

RecDumpRecords(RECT_NULL, collect_callback, &entries);

bool found = false;

for (const auto &entry : entries) {
if (entry.name == test_name) {
found = true;
CHECK(entry.rec_type == RECT_PLUGIN);
CHECK(entry.registered == 1);
CHECK(entry.data_type == RECD_STRING);
CHECK(entry.string_value == test_value);
break;
}
}

REQUIRE(found);
}