Skip to content

Commit b8a48ff

Browse files
khanayan123claude
andcommitted
fix: address review feedback from xlamorlette
- Rename constructor params to match member names - Rename rid to session_rid in session header tests - Use 3-param TracerSignature constructor where root_session == runtime_id Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d47c926 commit b8a48ff

4 files changed

Lines changed: 28 additions & 44 deletions

File tree

include/datadog/tracer_signature.h

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,17 @@ struct TracerSignature {
4141
StringView library_language_version;
4242

4343
TracerSignature() = delete;
44-
TracerSignature(RuntimeID id, std::string service, std::string environment)
45-
: TracerSignature(id, id.string(), std::move(service),
46-
std::move(environment)) {}
47-
TracerSignature(RuntimeID id, std::string root_session, std::string service,
48-
std::string environment)
49-
: runtime_id(id),
50-
root_session_id(std::move(root_session)),
51-
default_service(std::move(service)),
52-
default_environment(std::move(environment)),
44+
TracerSignature(RuntimeID runtime_id, std::string default_service,
45+
std::string default_environment)
46+
: TracerSignature(runtime_id, runtime_id.string(),
47+
std::move(default_service),
48+
std::move(default_environment)) {}
49+
TracerSignature(RuntimeID runtime_id, std::string root_session_id,
50+
std::string default_service, std::string default_environment)
51+
: runtime_id(runtime_id),
52+
root_session_id(std::move(root_session_id)),
53+
default_service(std::move(default_service)),
54+
default_environment(std::move(default_environment)),
5355
library_version(tracer_version),
5456
library_language("cpp"),
5557
library_language_version(DD_TRACE_STRINGIFY(__cplusplus), 6) {}

test/remote_config/test_remote_config.cpp

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,7 @@ auto logger = std::make_shared<NullLogger>();
4848
REMOTE_CONFIG_TEST("initial state payload") {
4949
// Verify the initial payload structure for a remote configuration instance.
5050
auto runtime_id = RuntimeID::generate();
51-
const TracerSignature tracer_signature{
52-
/* runtime_id = */ runtime_id,
53-
/* root_session_id = */ runtime_id.string(),
54-
/* service = */ "testsvc",
55-
/* environment = */ "test"};
51+
const TracerSignature tracer_signature{runtime_id, "testsvc", "test"};
5652

5753
auto tracing_listener = std::make_shared<FakeListener>();
5854
tracing_listener->products = rc::product::APM_TRACING;
@@ -95,11 +91,7 @@ REMOTE_CONFIG_TEST("initial state payload") {
9591

9692
REMOTE_CONFIG_TEST("response processing") {
9793
auto runtime_id = RuntimeID::generate();
98-
const TracerSignature tracer_signature{
99-
/* runtime_id = */ runtime_id,
100-
/* root_session_id = */ runtime_id.string(),
101-
/* service = */ "testsvc",
102-
/* environment = */ "test"};
94+
const TracerSignature tracer_signature{runtime_id, "testsvc", "test"};
10395

10496
SECTION("ill formatted input",
10597
"inputs not following the Remote Configuration JSON schema should "

test/telemetry/test_telemetry.cpp

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,7 @@ TELEMETRY_IMPLEMENTATION_TEST("Tracer telemetry lifecycle") {
103103
auto scheduler = std::make_shared<FakeEventScheduler>();
104104

105105
auto runtime_id = RuntimeID::generate();
106-
const TracerSignature tracer_signature{
107-
/* runtime_id = */ runtime_id,
108-
/* root_session_id = */ runtime_id.string(),
109-
/* service = */ "testsvc",
110-
/* environment = */ "test"};
106+
const TracerSignature tracer_signature{runtime_id, "testsvc", "test"};
111107

112108
auto url = HTTPClient::URL::parse("http://localhost:8000");
113109

@@ -375,41 +371,43 @@ TELEMETRY_IMPLEMENTATION_TEST("session ID headers") {
375371
auto url = HTTPClient::URL::parse("http://localhost:8000");
376372

377373
SECTION("root process: DD-Session-ID present, DD-Root-Session-ID absent") {
378-
auto rid = RuntimeID::generate();
379-
const TracerSignature sig{rid, rid.string(), "testsvc", "test"};
374+
auto session_rid = RuntimeID::generate();
375+
const TracerSignature sig{session_rid, "testsvc", "test"};
380376

381377
Telemetry telemetry{*finalize_config(), sig, logger, client,
382378
scheduler, *url};
383379

384380
auto it = client->request_headers.items.find("DD-Session-ID");
385381
REQUIRE(it != client->request_headers.items.end());
386-
CHECK(it->second == rid.string());
382+
CHECK(it->second == session_rid.string());
387383

388384
CHECK(client->request_headers.items.find("DD-Root-Session-ID") ==
389385
client->request_headers.items.end());
390386
}
391387

392388
SECTION("child process: DD-Root-Session-ID present when different") {
393-
auto rid = RuntimeID::generate();
389+
auto session_rid = RuntimeID::generate();
394390
auto root_rid = RuntimeID::generate();
395-
const TracerSignature sig{rid, root_rid.string(), "testsvc", "test"};
391+
const TracerSignature sig{session_rid, root_rid.string(), "testsvc",
392+
"test"};
396393

397394
Telemetry telemetry{*finalize_config(), sig, logger, client,
398395
scheduler, *url};
399396

400397
auto session_it = client->request_headers.items.find("DD-Session-ID");
401398
REQUIRE(session_it != client->request_headers.items.end());
402-
CHECK(session_it->second == rid.string());
399+
CHECK(session_it->second == session_rid.string());
403400

404401
auto root_it = client->request_headers.items.find("DD-Root-Session-ID");
405402
REQUIRE(root_it != client->request_headers.items.end());
406403
CHECK(root_it->second == root_rid.string());
407404
}
408405

409406
SECTION("heartbeat includes session headers") {
410-
auto rid = RuntimeID::generate();
407+
auto session_rid = RuntimeID::generate();
411408
auto root_rid = RuntimeID::generate();
412-
const TracerSignature sig{rid, root_rid.string(), "testsvc", "test"};
409+
const TracerSignature sig{session_rid, root_rid.string(), "testsvc",
410+
"test"};
413411

414412
Telemetry telemetry{*finalize_config(), sig, logger, client,
415413
scheduler, *url};
@@ -419,7 +417,7 @@ TELEMETRY_IMPLEMENTATION_TEST("session ID headers") {
419417

420418
auto session_it = client->request_headers.items.find("DD-Session-ID");
421419
REQUIRE(session_it != client->request_headers.items.end());
422-
CHECK(session_it->second == rid.string());
420+
CHECK(session_it->second == session_rid.string());
423421

424422
auto root_it = client->request_headers.items.find("DD-Root-Session-ID");
425423
REQUIRE(root_it != client->request_headers.items.end());
@@ -439,11 +437,7 @@ TELEMETRY_IMPLEMENTATION_TEST("Tracer telemetry API") {
439437
auto scheduler = std::make_shared<FakeEventScheduler>();
440438

441439
auto runtime_id = RuntimeID::generate();
442-
const TracerSignature tracer_signature{
443-
/* runtime_id = */ runtime_id,
444-
/* root_session_id = */ runtime_id.string(),
445-
/* service = */ "testsvc",
446-
/* environment = */ "test"};
440+
const TracerSignature tracer_signature{runtime_id, "testsvc", "test"};
447441

448442
auto url = HTTPClient::URL::parse("http://localhost:8000");
449443

@@ -1045,11 +1039,7 @@ TELEMETRY_IMPLEMENTATION_TEST("Tracer telemetry configuration") {
10451039
auto scheduler = std::make_shared<FakeEventScheduler>();
10461040

10471041
auto runtime_id = RuntimeID::generate();
1048-
const TracerSignature tracer_signature{
1049-
/* runtime_id = */ runtime_id,
1050-
/* root_session_id = */ runtime_id.string(),
1051-
/* service = */ "testsvc",
1052-
/* environment = */ "test"};
1042+
const TracerSignature tracer_signature{runtime_id, "testsvc", "test"};
10531043

10541044
auto url = HTTPClient::URL::parse("http://localhost:8000");
10551045

test/test_datadog_agent.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ DATADOG_AGENT_TEST("Remote Configuration") {
200200
REQUIRE(finalized);
201201

202202
auto rid = RuntimeID::generate();
203-
const TracerSignature signature(rid, rid.string(), "testsvc", "test");
203+
const TracerSignature signature(rid, "testsvc", "test");
204204

205205
// TODO: set telemetry mock
206206
auto config_manager = std::make_shared<ConfigManager>(*finalized);

0 commit comments

Comments
 (0)