Skip to content

Commit c31eef6

Browse files
dgoffredodgoffredo
committed
New env vars:
- DD_TRACE_PROPAGATION_STYLE - DD_TRACE_PROPAGATION_STYLE_INJECT - DD_TRACE_PROPAGATION_STYLE_EXTRACT
1 parent accbb7e commit c31eef6

File tree

4 files changed

+242
-30
lines changed

4 files changed

+242
-30
lines changed

‎src/datadog/error.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ struct Error {
6767
INVALID_DOUBLE = 42,
6868
MISSING_TRACE_ID = 43,
6969
ENVOY_HTTP_CLIENT_FAILURE = 44,
70+
MULTIPLE_PROPAGATION_STYLE_ENVIRONMENT_VARIABLES = 45,
7071
};
7172

7273
Code code;

‎src/datadog/optional.h‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
#include <optional>
3434
#endif // defined DD_USE_ABSEIL_FOR_ENVOY
3535

36+
#include <utility> // std::forward
37+
3638
namespace datadog {
3739
namespace tracing {
3840

@@ -46,5 +48,17 @@ using Optional = std::optional<Value>;
4648
inline constexpr auto nullopt = std::nullopt;
4749
#endif // defined DD_USE_ABSEIL_FOR_ENVOY
4850

51+
// Return the first non-null argument value. The last argument must not be
52+
// `Optional`.
53+
template <typename Value>
54+
auto value_or(Value&& value) {
55+
return std::forward<Value>(value);
56+
}
57+
58+
template <typename Value, typename... Rest>
59+
auto value_or(Optional<Value> maybe, Rest&&... rest) {
60+
return maybe.value_or(value_or(rest...));
61+
}
62+
4963
} // namespace tracing
5064
} // namespace datadog

‎src/datadog/tracer_config.cpp‎

Lines changed: 114 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "cerr_logger.h"
1111
#include "datadog_agent.h"
1212
#include "environment.h"
13+
#include "json.hpp"
1314
#include "null_collector.h"
1415
#include "parse_util.h"
1516
#include "string_view.h"
@@ -121,40 +122,127 @@ Expected<std::unordered_map<std::string, std::string>> parse_tags(
121122
return tags;
122123
}
123124

125+
// Return a `PropagationStyles` parsed from the specified `env_var`.
126+
// If `env_var` is not in the environment, return `nullopt`.
127+
// If an error occurs, throw an `Error`.
128+
Optional<PropagationStyles> styles_from_env(environment::Variable env_var) {
129+
const auto styles_env = lookup(env_var);
130+
if (!styles_env) {
131+
return {};
132+
}
133+
134+
auto styles = parse_propagation_styles(*styles_env);
135+
if (auto *error = styles.if_error()) {
136+
std::string prefix;
137+
prefix += "Unable to parse ";
138+
append(prefix, name(env_var));
139+
prefix += " environment variable: ";
140+
throw error->with_prefix(prefix);
141+
}
142+
return *styles;
143+
}
144+
145+
std::string json_quoted(StringView text) {
146+
std::string unquoted;
147+
assign(unquoted, text);
148+
return nlohmann::json(unquoted).dump();
149+
}
150+
124151
Expected<void> finalize_propagation_styles(FinalizedTracerConfig &result,
125-
const TracerConfig &config) {
126-
result.extraction_styles = config.extraction_styles;
127-
if (auto styles_env = lookup(environment::DD_PROPAGATION_STYLE_EXTRACT)) {
128-
auto styles = parse_propagation_styles(*styles_env);
129-
if (auto *error = styles.if_error()) {
130-
std::string prefix;
131-
prefix += "Unable to parse ";
132-
append(prefix, name(environment::DD_PROPAGATION_STYLE_EXTRACT));
133-
prefix += " environment variable: ";
134-
return error->with_prefix(prefix);
152+
const TracerConfig &config,
153+
Logger &logger) {
154+
namespace env = environment;
155+
// Print a warning if a questionable combination of environment variables is
156+
// defined.
157+
const auto ts = env::DD_TRACE_PROPAGATION_STYLE;
158+
const auto tse = env::DD_TRACE_PROPAGATION_STYLE_EXTRACT;
159+
const auto se = env::DD_PROPAGATION_STYLE_EXTRACT;
160+
const auto tsi = env::DD_TRACE_PROPAGATION_STYLE_INJECT;
161+
const auto si = env::DD_PROPAGATION_STYLE_INJECT;
162+
// clang-format off
163+
/*
164+
ts tse se tsi si
165+
--- --- --- --- ---
166+
ts | x warn warn warn warn
167+
|
168+
tse | x x warn ok ok
169+
|
170+
se | x x x ok ok
171+
|
172+
tsi | x x x x warn
173+
|
174+
si | x x x x x
175+
*/
176+
// In each pair, the first would be overridden by the second.
177+
const std::pair<env::Variable, env::Variable> questionable_combinations[] = {
178+
{ts, tse}, {ts, se}, {ts, tsi}, {ts, si},
179+
180+
{se, tse}, /* ok */ /* ok */
181+
182+
/* ok */ /* ok */
183+
184+
{si, tsi},
185+
};
186+
// clang-format on
187+
188+
const auto warn_message = [](StringView name, StringView value,
189+
StringView name_override,
190+
StringView value_override) {
191+
std::string message;
192+
message += "Both the environment variables ";
193+
append(message, name);
194+
message += "=";
195+
message += json_quoted(value);
196+
message += " and ";
197+
append(message, name_override);
198+
message += "=";
199+
message += json_quoted(value_override);
200+
message += " are defined. ";
201+
append(message, name_override);
202+
message += " will take precedence.";
203+
return message;
204+
};
205+
206+
for (const auto &[var, var_override] : questionable_combinations) {
207+
const auto value = lookup(var);
208+
if (!value) {
209+
continue;
210+
}
211+
const auto value_override = lookup(var_override);
212+
if (!value_override) {
213+
continue;
135214
}
136-
result.extraction_styles = *styles;
215+
const auto var_name = name(var);
216+
const auto var_name_override = name(var_override);
217+
logger.log_error(Error{
218+
Error::MULTIPLE_PROPAGATION_STYLE_ENVIRONMENT_VARIABLES,
219+
warn_message(var_name, *value, var_name_override, *value_override)});
137220
}
138221

139-
result.injection_styles = config.injection_styles;
140-
if (auto styles_env = lookup(environment::DD_PROPAGATION_STYLE_INJECT)) {
141-
auto styles = parse_propagation_styles(*styles_env);
142-
if (auto *error = styles.if_error()) {
143-
std::string prefix;
144-
prefix += "Unable to parse ";
145-
append(prefix, name(environment::DD_PROPAGATION_STYLE_INJECT));
146-
prefix += " environment variable: ";
147-
return error->with_prefix(prefix);
148-
}
149-
result.injection_styles = *styles;
222+
// Parse the propagation styles from the configuration and/or from the
223+
// environment.
224+
// Exceptions make this section simpler.
225+
try {
226+
const auto global_styles = styles_from_env(env::DD_TRACE_PROPAGATION_STYLE);
227+
result.extraction_styles =
228+
value_or(styles_from_env(env::DD_TRACE_PROPAGATION_STYLE_EXTRACT),
229+
styles_from_env(env::DD_PROPAGATION_STYLE_EXTRACT),
230+
global_styles, config.extraction_styles);
231+
result.injection_styles =
232+
value_or(styles_from_env(env::DD_TRACE_PROPAGATION_STYLE_INJECT),
233+
styles_from_env(env::DD_PROPAGATION_STYLE_INJECT),
234+
global_styles, config.injection_styles);
235+
} catch (Error &error) {
236+
return std::move(error);
150237
}
151238

152239
if (!result.extraction_styles.datadog && !result.extraction_styles.b3 &&
153240
!result.extraction_styles.none) {
154241
return Error{Error::MISSING_SPAN_EXTRACTION_STYLE,
155242
"At least one extraction style must be specified."};
156-
} else if (!result.injection_styles.datadog && !result.injection_styles.b3 &&
157-
!result.injection_styles.none) {
243+
}
244+
if (!result.injection_styles.datadog && !result.injection_styles.b3 &&
245+
!result.injection_styles.none) {
158246
return Error{Error::MISSING_SPAN_INJECTION_STYLE,
159247
"At least one injection style must be specified."};
160248
}
@@ -236,7 +324,8 @@ Expected<FinalizedTracerConfig> finalize_config(const TracerConfig &config) {
236324
return std::move(span_sampler_config.error());
237325
}
238326

239-
auto maybe_error = finalize_propagation_styles(result, config);
327+
auto maybe_error =
328+
finalize_propagation_styles(result, config, *result.logger);
240329
if (!maybe_error) {
241330
return maybe_error.error();
242331
}

‎test/tracer_config.cpp‎

Lines changed: 113 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <datadog/tracer_config.h>
77

88
#include <cmath>
9+
#include <cstddef>
910
#include <cstdlib>
1011
#include <filesystem>
1112
#include <fstream>
@@ -985,6 +986,20 @@ TEST_CASE("TracerConfig propagation styles") {
985986
TracerConfig config;
986987
config.defaults.service = "testsvc";
987988

989+
SECTION("DD_TRACE_PROPAGATION_STYLE overrides defaults") {
990+
const EnvGuard guard{"DD_TRACE_PROPAGATION_STYLE", "B3"};
991+
auto finalized = finalize_config(config);
992+
REQUIRE(finalized);
993+
994+
REQUIRE(!finalized->injection_styles.datadog);
995+
REQUIRE(finalized->injection_styles.b3);
996+
REQUIRE(!finalized->injection_styles.none);
997+
998+
REQUIRE(!finalized->extraction_styles.datadog);
999+
REQUIRE(finalized->extraction_styles.b3);
1000+
REQUIRE(!finalized->extraction_styles.none);
1001+
}
1002+
9881003
SECTION("injection_styles") {
9891004
SECTION("defaults to just Datadog") {
9901005
auto finalized = finalize_config(config);
@@ -1001,9 +1016,31 @@ TEST_CASE("TracerConfig propagation styles") {
10011016
REQUIRE(finalized.error().code == Error::MISSING_SPAN_INJECTION_STYLE);
10021017
}
10031018

1004-
SECTION("DD_PROPAGATION_STYLE_INJECT") {
1019+
SECTION("DD_TRACE_PROPAGATION_STYLE_INJECT") {
10051020
SECTION("overrides injection_styles") {
1006-
const EnvGuard guard{"DD_PROPAGATION_STYLE_INJECT", "B3"};
1021+
const EnvGuard guard{"DD_TRACE_PROPAGATION_STYLE_INJECT", "B3"};
1022+
auto finalized = finalize_config(config);
1023+
REQUIRE(finalized);
1024+
REQUIRE(!finalized->injection_styles.datadog);
1025+
REQUIRE(finalized->injection_styles.b3);
1026+
REQUIRE(!finalized->injection_styles.none);
1027+
}
1028+
1029+
SECTION("overrides DD_PROPAGATION_STYLE_INJECT") {
1030+
const EnvGuard guard1{"DD_TRACE_PROPAGATION_STYLE_INJECT", "B3"};
1031+
const EnvGuard guard2{"DD_PROPAGATION_STYLE_INJECT", "Datadog"};
1032+
config.logger = std::make_shared<MockLogger>(); // suppress warning
1033+
auto finalized = finalize_config(config);
1034+
REQUIRE(finalized);
1035+
REQUIRE(!finalized->injection_styles.datadog);
1036+
REQUIRE(finalized->injection_styles.b3);
1037+
REQUIRE(!finalized->injection_styles.none);
1038+
}
1039+
1040+
SECTION("overrides DD_TRACE_PROPAGATION_STYLE") {
1041+
const EnvGuard guard1{"DD_TRACE_PROPAGATION_STYLE_INJECT", "B3"};
1042+
const EnvGuard guard2{"DD_TRACE_PROPAGATION_STYLE", "Datadog"};
1043+
config.logger = std::make_shared<MockLogger>(); // suppress warning
10071044
auto finalized = finalize_config(config);
10081045
REQUIRE(finalized);
10091046
REQUIRE(!finalized->injection_styles.datadog);
@@ -1045,7 +1082,7 @@ TEST_CASE("TracerConfig propagation styles") {
10451082
CAPTURE(test_case.line);
10461083
CAPTURE(test_case.env_value);
10471084

1048-
const EnvGuard guard{"DD_PROPAGATION_STYLE_INJECT",
1085+
const EnvGuard guard{"DD_TRACE_PROPAGATION_STYLE_INJECT",
10491086
test_case.env_value};
10501087
auto finalized = finalize_config(config);
10511088
if (test_case.expected_error) {
@@ -1083,9 +1120,31 @@ TEST_CASE("TracerConfig propagation styles") {
10831120
REQUIRE(finalized.error().code == Error::MISSING_SPAN_EXTRACTION_STYLE);
10841121
}
10851122

1086-
SECTION("DD_PROPAGATION_STYLE_EXTRACT") {
1123+
SECTION("DD_TRACE_PROPAGATION_STYLE_EXTRACT") {
10871124
SECTION("overrides extraction_styles") {
1088-
const EnvGuard guard{"DD_PROPAGATION_STYLE_EXTRACT", "B3"};
1125+
const EnvGuard guard{"DD_TRACE_PROPAGATION_STYLE_EXTRACT", "B3"};
1126+
auto finalized = finalize_config(config);
1127+
REQUIRE(finalized);
1128+
REQUIRE(!finalized->extraction_styles.datadog);
1129+
REQUIRE(finalized->extraction_styles.b3);
1130+
REQUIRE(!finalized->extraction_styles.none);
1131+
}
1132+
1133+
SECTION("overrides DD_PROPAGATION_STYLE_EXTRACT") {
1134+
const EnvGuard guard1{"DD_TRACE_PROPAGATION_STYLE_EXTRACT", "B3"};
1135+
const EnvGuard guard2{"DD_PROPAGATION_STYLE_EXTRACT", "Datadog"};
1136+
config.logger = std::make_shared<MockLogger>(); // suppress warning
1137+
auto finalized = finalize_config(config);
1138+
REQUIRE(finalized);
1139+
REQUIRE(!finalized->extraction_styles.datadog);
1140+
REQUIRE(finalized->extraction_styles.b3);
1141+
REQUIRE(!finalized->extraction_styles.none);
1142+
}
1143+
1144+
SECTION("overrides DD_TRACE_PROPAGATION_STYLE") {
1145+
const EnvGuard guard1{"DD_TRACE_PROPAGATION_STYLE_EXTRACT", "B3"};
1146+
const EnvGuard guard2{"DD_TRACE_PROPAGATION_STYLE", "Datadog"};
1147+
config.logger = std::make_shared<MockLogger>(); // suppress warning
10891148
auto finalized = finalize_config(config);
10901149
REQUIRE(finalized);
10911150
REQUIRE(!finalized->extraction_styles.datadog);
@@ -1104,4 +1163,53 @@ TEST_CASE("TracerConfig propagation styles") {
11041163
}
11051164
}
11061165
}
1166+
1167+
SECTION("warn if one env var overrides another") {
1168+
const auto logger = std::make_shared<MockLogger>();
1169+
config.logger = logger;
1170+
const auto ts = "DD_TRACE_PROPAGATION_STYLE";
1171+
const auto tse = "DD_TRACE_PROPAGATION_STYLE_EXTRACT";
1172+
const auto se = "DD_PROPAGATION_STYLE_EXTRACT";
1173+
const auto tsi = "DD_TRACE_PROPAGATION_STYLE_INJECT";
1174+
const auto si = "DD_PROPAGATION_STYLE_INJECT";
1175+
const char* const vars[] = {ts, tse, se, tsi, si};
1176+
constexpr auto n = sizeof(vars) / sizeof(vars[0]);
1177+
// clang-format off
1178+
const bool x = false; // ignored values
1179+
const bool expect_warning[n][n] = {
1180+
// ts tse se tsi si
1181+
// --- --- --- --- ---
1182+
/* ts */{ x, true, true, true, true },
1183+
1184+
/* tse */{ x, x, true, false, false },
1185+
1186+
/* se */{ x, x, x, false, false },
1187+
1188+
/* tsi */{ x, x, x, x, true },
1189+
1190+
/* si */{ x, x, x, x, x },
1191+
};
1192+
// clang-format on
1193+
for (std::size_t i = 0; i < n; ++i) {
1194+
for (std::size_t j = i + 1; j < n; ++j) {
1195+
CAPTURE(i);
1196+
CAPTURE(vars[i]);
1197+
CAPTURE(j);
1198+
CAPTURE(vars[j]);
1199+
CAPTURE(expect_warning[i][j]);
1200+
const EnvGuard guard1{vars[i], "B3"};
1201+
const EnvGuard guard2{vars[j], "B3"};
1202+
const auto finalized_config = finalize_config(config);
1203+
REQUIRE(finalized_config);
1204+
if (expect_warning[i][j]) {
1205+
REQUIRE(logger->error_count() == 1);
1206+
REQUIRE(logger->first_error().code ==
1207+
Error::MULTIPLE_PROPAGATION_STYLE_ENVIRONMENT_VARIABLES);
1208+
} else {
1209+
REQUIRE(logger->error_count() == 0);
1210+
}
1211+
logger->entries.clear();
1212+
}
1213+
}
1214+
}
11071215
}

0 commit comments

Comments
 (0)